feat: Reintroduce default bin finder constructor #1197
Conversation
stephenswat
added
Component - Core
In Acts version 15.0.0, we see support being added for dynamic bins in the bin finder. However, this also means that we no longer have useful default constructors for this class. This has caused me a few issues in terms of backwards compatibility. I think it would be a good idea to reintroduce some sane default configuration for the bin finder, and I discussed with @LuisFelipeCoelho that having an empty list of z neighbours with a single phi neighbour would be a good default.
stephenswat
force-pushed
the
feat/binfinder_default_constructor
branch
from
5427364
to
8cea3ba
Compare
Codecov Report
@@ Coverage Diff @@
## main #1197 +/- ##
=======================================
Coverage 47.75% 47.75%
=======================================
Files 360 360
Lines 18616 18616
Branches 8774 8774
=======================================
Hits 8890 8890
Misses 3674 3674
Partials 6052 6052
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
|
Format fails. |
|
On code that I didn't touch 😢 |
|
That is pretty weird. The CI is not setup to only format changes, this was merged the way it's formatted now in #1038, and there clang-format was happy... and has been on every commit to main since then. But here it fails, and locally on my machine clang-format also reformats that file... What the heck? |
|
That's what I thought as well. 😄 As much as I dislike staging and committing unrelated changes like this it's really no problem. |
There was a problem hiding this comment.
@LuisFelipeCoelho had approved before, no changes since then other than formatting.
In Acts version 15.0.0, we see support being added for dynamic bins in the bin finder. However, this also means that we no longer have useful default constructors for this class. This has caused me a few issues in terms of backwards compatibility. I think it would be a good idea to reintroduce some sane default configuration for the bin finder, and I discussed with @LuisFelipeCoelho that having an empty list of z neighbours with a single φ neighbour would be a good default.