feat: use algorithms properties in CalorimeterClusterRecoCoG (2) #1165
Conversation
github-actions
bot
added
topic: calorimetry
| [this, key = key](auto&& val) { | ||
| using T = std::decay_t<decltype(val)>; | ||
| if constexpr (std::is_fundamental_v<T> | ||
| || std::is_same_v<T, std::string>) { |
There was a problem hiding this comment.
Vector is now also ok, I think.
|
It is curious that this introduces the kind of diffs (e.g. https://github.com/eic/EICrecon/actions/runs/7072810354/job/19252231634#step:15:82) that are more typically assocated with double/float conversions. So either before or after there may be a conversion that shouldn't happen. |
| std::visit( | ||
| [this, key = key, ¶meters](auto&& val) { | ||
| using T = std::decay_t<decltype(val)>; | ||
| if constexpr (std::is_fundamental_v<T> || std::is_same_v<T, std::string>) { |
There was a problem hiding this comment.
This is incomplete because JParameterManager can parse the algorithms::Property of type T = std::vector<U> where U is a fundamental type or std::string. However, we can't use
| if constexpr (std::is_fundamental_v<T> || std::is_same_v<T, std::string>) { | |
| if constexpr (std::is_fundamental_v<T> || std::is_same_v<T, std::string> | |
| || std::is_same_v<std::vector<typename T::value_type, T> && std::is_fundamental_v<typename T::value_type>) { |
because it tries to apply this to the fundamental types too, and fails due to T::value_type not existing for e.g. T = bool. I'm not sure yet how to solve this.
There was a problem hiding this comment.
@nathanwbrei @veprbl Maybe you have some insights on how to structure a type test here.
Actually, it's worse. Somewhere I am passing a reference to a copy of the property value in the algorithm, so the algorithm still operates with the original default... Back to the drawing board. |
wdconinc
force-pushed
the
algorithms-properties-2
branch
from
0211205
to
373aad8
Compare
wdconinc
force-pushed
the
algorithms-properties-2
branch
from
373aad8
to
24be6e3
Compare
Briefly, what does this PR introduce?
This PR converts CalorimeterClusterRecoCoG to using properties defined as algorithms::Property. This is the second attempt at #1154, now based on JOmniFactory and ParameterRef.
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
No.
Does this PR change default behavior?
No.