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

Add extended mag field map #1632

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Add extended mag field map #1632

wants to merge 1 commit into from

Conversation

tvami
Copy link
Member

@tvami tvami commented Feb 22, 2025

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

What are the issues that this addresses?

Resolves #1562
(technically, but I'm not sure I like the solution)

I extended the grid to

    x_range = np.arange(-500.0, 500.0, 5.0)
    y_range = np.arange(-500.0, 500.0, 5.0)
    z_range = np.arange(-1500.0, 2500.0, 5.0)

and added zeros everywhere where the is no value already.
I also tried to just add zeros to the edges of the cude: this didnt work, since the bin size is stepping and it's not available.

Check List

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

The ACTS msg about propagation failure is gone.

@tvami
Copy link
Member Author

tvami commented Feb 25, 2025

This lead to a lot of differences all over
https://github.com/LDMX-Software/ldmx-sw/actions/runs/13469851573?pr=1632

@tvami
Copy link
Member Author

tvami commented Feb 25, 2025

I'm pretty confused about a few things I see:

I'm looking at signal now, since that should be more affected.
[plots are normalized to unit area]

The incident electron pT is wildly different:
Screenshot 2025-02-25 at 15 30 38

But even the recoil goes higher:
Screenshot 2025-02-25 at 15 31 46

The recoil ele at the ECAL face is now deflected in pos X
Screenshot 2025-02-25 at 15 33 58

But the y distribution looks more smooth
Screenshot 2025-02-25 at 15 35 05

The effect on the BDT is good too, we gain about 20% signal (because the electron becomes fiducial in more cases)
Screenshot 2025-02-25 at 15 36 06

Things in HCAL changed a lot, but it all goes down to seeing less stuff in the side HCAL
Screenshot 2025-02-25 at 15 40 03

Which then improves the HCAL veto performance (a tiny bit)
Screenshot 2025-02-25 at 15 41 03

None of the above depends on the ACTS tracking, it's all truth.

Now ACTS tracking

Recoil d0:
Screenshot 2025-02-25 at 15 44 49

Pulls at the ECAL improve:
Screenshot 2025-02-25 at 15 45 25

I guess we lost more hits in the end of the recoil
Screenshot 2025-02-25 at 15 46 22

The number of tracks is not going into the direction I wanted in CKF... but I guess given how off the d0 etc is this is not surprising.
Screenshot 2025-02-25 at 15 50 06

Now the same in GAS looks improved
Screenshot 2025-02-25 at 15 52 26

The truth phi is shifted, so it's not just ACTS settings
Screenshot 2025-02-25 at 15 53 30

@tvami
Copy link
Member Author

tvami commented Feb 25, 2025

I should also say that in ECAL PN gold the text "PROP ERROR Step failed with MagneticFieldError:1: Interpolation out of bounds was requested" appears 129 times, in the logs after this PR all that is gone... so at least the direct problem is gone haha, but may have introduced other problems. I'm not sure what to take from the plots above.

@EBerzin
Copy link
Contributor

EBerzin commented Mar 7, 2025

@tvami I think the issue might be the order the Bfield values are listed in file. From looking at how the field is read in MagneticFieldMap3D.cxx, it looks like it assumes that the points will be provided in either strict increasing or decreasing order. The interpolation in GetFieldValue() is then based on the grid index. In the extended field map, this is violated here, for example, where the x and y positions jump from 495 to -250 and -70. I think for things to make sense, the existing field values need to be filled in to preserve position ordering, rather than at the end of the file, where they are now.

495.0 495.0 2485.0 1e-11 1e-11 1e-11
495.0 495.0 2490.0 1e-11 1e-11 1e-11
495.0 495.0 2495.0 1e-11 1e-11 1e-11
-250.0 -70.0 -1495.0 2.509e-07 -4.39e-06 1.031e-06
-250.0 -70.0 -1490.0 2.571e-07 -4.457e-06 1.076e-06
-250.0 -70.0 -1485.0 2.633e-07 -4.525e-06 1.121e-06

@tomeichlersmith
Copy link
Member

(I would love for you to commit whatever code you are using to evolve the map into MagFieldMap/utils or something as well. I have a feeling we will need to do something like this again...)

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.

MagneticFieldError with propagation interpolation
3 participants