Updated CLUE clustering to pull RoC values from CSV file, if available.#2053
Open
cjbarton151 wants to merge 2 commits into
Open
Updated CLUE clustering to pull RoC values from CSV file, if available.#2053cjbarton151 wants to merge 2 commits into
cjbarton151 wants to merge 2 commits into
Conversation
Contributor
Validation ResultsSome validation samples failed! ❌
|
7beaa23 to
34831bc
Compare
Member
|
/run-validation |
Contributor
|
The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/26490893162. |
Contributor
Validation ResultsAll validation samples passed! ✅
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I am updating ldmx-sw, here are the details.
What are the issues that this addresses?
This resolves #1885
When calculating... Something (not sure what and it doesn't say in the code), the CLUE algorithm has been relying on a hardcoded array of containment radii defined in CLUE.h. These values are outdated and, like most hardcoded values, at risk of becoming outdated again even if we update them.
With this PR, if possible, the hardcoded array will be replaced with values pulled from a CSV file stored in Ecal/Data. To do this, the path to the selected CSV file is passed through the EcalClusterProducer, just like the rest of the parameters used in CLUE. If there is no CSV file path defined, the program defaults to using the hardcoded values. If the file path is somehow faulty, the program exits with an error. I considered downgrading this to a warning, followed by defaulting to the hardcoded values, but I think an exception is more appropriate.
Only two disclaimers this time:
(as an aside: it turns out that there's some thorny issue related to localization that messes up
stodandstofwhen operating in a country where decimal places are denoted by ',' instead of '.' and I spent way too much time diving into this rabbit hole)Check List
Ran without crashing, RoC values were updated (when setting ECalClusterProducer.nbr_of_layers to a value >1, which I don't think any of the CI tests do anyway)