Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize ecal biasing factor for 8 GeV #1566

Draft
wants to merge 8 commits into
base: trunk
Choose a base branch
from

Conversation

tvami
Copy link
Member

@tvami tvami commented Feb 3, 2025

I am updating ldmx-sw, here are the details.

What are the issues that this addresses?

Resolves #1535

Check List

  • I successfully compiled ldmx-sw with my developments
  • I ran my developments and the following shows that they are successful.

See #1535 (comment)

@tvami tvami marked this pull request as draft February 3, 2025 15:41
@tvami tvami marked this pull request as ready for review February 3, 2025 15:54
@tvami tvami marked this pull request as draft February 3, 2025 15:54
@tvami
Copy link
Member Author

tvami commented Feb 4, 2025

I'm gonna use this PR to test #1569

@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@tvami tvami marked this pull request as ready for review February 4, 2025 15:51
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@tvami
Copy link
Member Author

tvami commented Feb 4, 2025

/run-validation

Copy link
Contributor

github-actions bot commented Feb 4, 2025

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/13140056694.

@LDMX-Software LDMX-Software deleted a comment from github-actions bot Feb 4, 2025
@tvami
Copy link
Member Author

tvami commented Feb 4, 2025

I think this works now!

@tvami
Copy link
Member Author

tvami commented Feb 4, 2025

I'll test if it also works in a draft mode

@tvami tvami marked this pull request as draft February 4, 2025 16:28
@tvami
Copy link
Member Author

tvami commented Feb 4, 2025

/run-validation

Copy link
Contributor

github-actions bot commented Feb 4, 2025

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/13140159042.

@tvami
Copy link
Member Author

tvami commented Feb 4, 2025

  Switched to a new branch 'iss1535-biasing-factor-optimization'
  branch 'iss1535-biasing-factor-optimization' set up to track 'origin/iss1535-biasing-factor-optimization'.

Yaay it does! Sorry everybody if you got more than than a dozen msg because of this!

@tvami tvami marked this pull request as ready for review February 10, 2025 21:56
@tvami
Copy link
Member Author

tvami commented Feb 10, 2025

I'm opening this for review, as we agreed today that this will not affect anything in the DR and is safe to merge

Copy link
Member

@tomeichlersmith tomeichlersmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk... do we want to wait and see if we are really over-biasing a high energy child-of-primary photon? I'm thinking that maybe there is a bug in our checks on which particles get biased...

@tvami tvami marked this pull request as ready for review February 17, 2025 19:50
@tvami tvami marked this pull request as draft February 17, 2025 19:50
@tvami
Copy link
Member Author

tvami commented Feb 17, 2025

this is the Z vertex with a factor of a 1000:
Screenshot 2025-02-17 at 14 13 22

@tvami
Copy link
Member Author

tvami commented Feb 17, 2025

Here is the rest of the KS-failed distributions:
change_bias_factor_1000-ecal_pn_recon_validation_plots.pdf

@bryngemark
Copy link
Contributor

With a factor 1000 it looks like we're getting about 10% more events in the first two bins than we used to (btw is the reference with 8 GeV/550, or 4 GeV/450?). I think this could still mean that we're getting a large enough xsc to trigger a warning but it is suppressed.

Did you get around to running with old_ecal? That should factor the low-Z materials out of the problem. If we can run without these materials, maybe finding the maximum warning-free biasing factor would be a good procedure to optimize without having to look at z.

@tvami
Copy link
Member Author

tvami commented Feb 18, 2025

(btw is the reference with 8 GeV/550, or 4 GeV/450?).

with ref to 8 GeV/550

Did you get around to running with old_ecal?

Not yet, but sounds good

@bryngemark
Copy link
Contributor

I suggest we discuss this at the sw dev meeting today and type up what we learn from that and the email thread with natalia in this issue afterwards.

@tvami tvami marked this pull request as ready for review February 20, 2025 20:20
@tvami
Copy link
Member Author

tvami commented Feb 20, 2025

Let's test with the new gold

@tvami
Copy link
Member Author

tvami commented Feb 21, 2025

Screenshot 2025-02-20 at 17 52 09 Screenshot 2025-02-20 at 17 52 33

@tvami
Copy link
Member Author

tvami commented Feb 21, 2025

So a little bit more interactions in the non-W part of the ecal, but it seems the with a factor of a 1000 it's not too different.

Reminder about the Z dist (now with the clean gold, and normalized plots with error bars):
Screenshot 2025-02-20 at 17 56 43

@tvami
Copy link
Member Author

tvami commented Feb 21, 2025

The rest of the plots that fail the KS (reminder the red is with 550 and the blue is with 1000)
bias-opt-ecal_pn_recon_validation_plots.pdf

@tvami tvami marked this pull request as draft February 22, 2025 04:54
@tvami tvami marked this pull request as ready for review February 22, 2025 04:54
@tvami tvami marked this pull request as draft February 22, 2025 04:55
@tvami
Copy link
Member Author

tvami commented Feb 25, 2025

Results in https://github.com/LDMX-Software/ldmx-sw/actions/runs/13469761175

The z with using biasing factor of 5000

Screenshot 2025-02-25 at 14 38 09

Material
Screenshot 2025-02-25 at 14 38 38

Volume
Screenshot 2025-02-25 at 14 38 57

So more is happening now in the non-W based elements, I guess this was expected

@tvami tvami marked this pull request as ready for review February 25, 2025 23:03
@tvami tvami marked this pull request as draft February 25, 2025 23:03
@tvami
Copy link
Member Author

tvami commented Feb 25, 2025

I'll check now how things look like for biasing factor of 1

@tvami tvami marked this pull request as ready for review March 3, 2025 16:13
@tvami tvami marked this pull request as draft March 3, 2025 16:13
@tvami tvami marked this pull request as ready for review March 5, 2025 02:18
@tvami tvami marked this pull request as draft March 5, 2025 02:18
@tvami
Copy link
Member Author

tvami commented Mar 5, 2025

I'll check now how things look like for biasing factor of 1

Ohh so with biasing factor of 1 the ecalPN takes longer than 6 hours to run, so github kills it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Biasing factor is too large" warning
3 participants