De-Template (Host) Track Fitting, main branch (2024.10.28.) #756
Conversation
krasznaa
added
build
krasznaa
force-pushed
the
DeTemplateTrackFitting-main-20241024
branch
2 times, most recently
from
502fec6
to
145d6e1
Compare
| namespace traccc::host { | ||
|
|
||
| /// Kalman filter based track fitting algorithm | ||
| class kf_algorithm : public algorithm<track_state_container_types::host( |
There was a problem hiding this comment.
This should be just called kalman_fitter instead of kf_algorithm
There was a problem hiding this comment.
The kalman_fitter fitter class still exists.
🤔 We could call it traccc::host::kalman_fitter_algorithm instead, but since traccc::host::ckf_algorithm already went through, I thought I'd go with this short name here as well.
There was a problem hiding this comment.
This is what is exposed to the user directly and naming it into kf_algorithm feels less obvious to me. kalman_fitting_algorithm might work better
ckf_algorithm probably need to be changed to ckf_finding_algorithm or sth else
| @@ -0,0 +1,38 @@ | |||
| /** TRACCC library, part of the ACTS project (R&D line) | |||
There was a problem hiding this comment.
I don't think defdet or teldet is very understandable to general users... how about
kalman_fitter_constant_field
kalman_fitter_constant_field_telescope ?
There was a problem hiding this comment.
I don't have strong feelings about these file names. But keep in mind that users will not be exposed to any of these names. These are just implementation details behind the scenes. That's why I went with relatively cryptic and short names. Because people who are not developers of this code, would not be exposed to these files anyway.
There was a problem hiding this comment.
I believe having more verbosity is going to be helpful for devs as well.
There was a problem hiding this comment.
We will keep expanding the list of this specialization for different detector and inhomogeneous field
Being cryptic makes the maintenance more difficult I guess
There was a problem hiding this comment.
As I said, I'm very open to naming these files differently. I'll make this change shortly.
krasznaa
force-pushed
the
DeTemplateTrackFitting-main-20241024
branch
3 times, most recently
from
47a7e27
to
6571488
Compare
It is a replacement for traccc::fitting_algorithm, without a templated API.
Updating all users to use traccc::host::kf_algorithm instread.
krasznaa
force-pushed
the
DeTemplateTrackFitting-main-20241024
branch
from
6571488
to
9b4ecee
Compare
This way clients can have access to the full details of the code if they want to, while also getting algorithms that would perform fitting in one very specific way.
|
|
Please have a look @paulgessinger. I now still have a specific algorithm interface for performing the fitting "in specific ways", but also expose the template code that would allow clients to do different things as well if they so wished. I think this design could be extended to the device code as well. 🤔 |
| /// | ||
| /// @return A container of the fitted track states | ||
| /// | ||
| template <typename fitter_t> |
There was a problem hiding this comment.
We should technically do this with a "fitter concept", but didn't want to pfaff with that in this PR. I'll be happy to let @stephenswat introduce the "fitter concept" anyway. 😄
This is the next step in my crusade to make the reconstruction algorithms use non-templated interfaces. 😄
In this I renamed
traccc::fitting_algorithmtotraccc::host::kf_algorithm, along exactly the same lines as what I did fortraccc::host::ckf_algorithmin #722.As before, one important aspect of this is to get the memory needs of the build under control. In the current
mainbranch we use the following amounts of memory for compiling a few representative source files:seq_example.cpp: 1.4 GBcpu/throughput_st.cpp: 700 MBThis PR introduces 3 new source files, and reduces the memory needs of the previous examples like:
kf_algorithm.cpp: 320 MBkf_algorithm_teldet_cfield.cpp: 475 MBkf_algorithm_defdet_cfield.cpp: 650 MBseq_example.cpp: 1.0 GBcpu/throughput_st.cpp: 575 MBClearly this is not the end of the road yet, but I'm fairly happy with what the PR does for the build's memory usage.