Add CLUE clustering #1699
Add CLUE clustering #1699
Conversation
git switch ella-dev-clustering git rebase trunk git switch 1411-add-clue-clustering git checkout ella-dev-clustering -- Ecal/ Ella Viirola originally authored these files while doing summer research into ECal clustering at Lund University Co-authored-by: Lysarina
… than the incident copied over from work done originally by Ella Viirola during Summer 2024 at Lund. Co-authored-by: Lysarina
|
@bryngemark The fact that the PR Validations pass completely leads me to believe that the issues you were observing before are not present (at least within the clean CI environment.) The total energy is appropriately shifted high and is not the discrete distribution you've shown before.
Since this distribution is cut off at higher energies, I'm re-running the |
There was a problem hiding this comment.
I skimmed thru quickly, I'll have a 2nd read-thru later, but two general comments:
- the variable names are not according to our coding rules
- there are
couts anddebugflags that should go away into the logging system (everything under debug should betracein our logging system)
|
@bryngemark I have confirmed that, locally and running from scratch, the IT overlay is working as expected. As a double check, I just looked at the
Without being able to replicate your issue, I would like to ask you to provide more detail on the environment and inputs you used to produce the plot in the original issue. What version of ldmx-sw produced the overlay events? What version produced the "main" events? What version ran overlay? Those sort of questions. If the environment is on SLAC's S3DF, I'd be happy to look at it directly as well if you give me the directory (and I have access to read that directory). |
needed to patch python parameter naming and now it works
|
that's great, @tomeichlersmith. i was getting hopeful when the validation pileup test went through. i ran on ella's dev branch (that the PR was made from) locally on my laptop. i confirmed that it was behaving differently than the then-current |
|
@tvami I have not moved all of the variables in the CLUE implementation to |
|
Hi! This might already be resolved or might not even be an issue, but as I wrote in my PR, the initial cluster producer used the WorkingCluster class, that took in EcalHits as pointers. However when I started out I was kind of a pointer noob so I created WorkingEcalCluster, which does essentially the same thing (with some additional improvements), but takes in references instead of pointers in the methods. I never had time to fix this (going back to pointers), so I just wanted to point this out again, in case this would cause some issues later down the line. Thanks again for taking this on @tomeichlersmith ! |
add a bit more documentation to the SD implementation
Getting factory updates to make sure things still work after trunk update.
I am copying over the CLUE clustering, additional analyzer and update to the SimCalorimeterHit that was originally developed in Summer 2024 to study ECal clustering at Lund by Ella Viirola. The visualization code based on Phoenix I plan to move into its own repository and the JSON-producers that prepare an event to be visualized I plan to put into EventDisplay.
What are the issues that this addresses?
This resolves #1411
Check List
@Lysarina originally did these developments and includes evidence of their success in the original PR #1416
To Do