Implement lumi dipole B mapping #527

dhevang · 2023-09-18T19:47:19Z

Briefly, what does this PR introduce?

Implement lumi dipole B mapping and modify source code to treat R-Z and X-Y-Z coordinate systems.

What kind of change does this PR introduce?

Bug fix (issue #__)
[x ] New feature (issue #__)
Documentation update
Other: __

Please check if this PR fulfills the following:

Tests for the changes have been added
Documentation has been added / updated
Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

Does this PR change default behavior?

…gnet.

Implement lumi dipole B maps.

for more information, see https://pre-commit.ci

dhevang · 2023-09-18T20:04:58Z

@wdconinc , in your code, FieldMapBrBz.cpp, which I used as the basis for this extension, you had a coordinate transformation here:

epic/src/FieldMapBrBz.cpp

Line 152 in 19ec3f3

auto p = trans_inv * ROOT::Math::XYZPoint(pos[0], pos[1], pos[2]);

and then a B field transformation (trans) here:

epic/src/FieldMapBrBz.cpp

Line 182 in 19ec3f3

auto B = trans * ROOT::Math::XYZPoint(Br * sin(phi), Br * cos(phi), Bz);

I'm not sure I understand the latter. To me, only the first one was needed. Anyway, the translation and rotation for the MARCO field were zero, so this had no effect. However, for the lumi dipole field, I have the translation active.

wdconinc · 2023-09-18T20:08:50Z

Can you rebase so this lists only the changes relevant to this PR under files? Probably just a matter of running git rebase origin/main in your branch, then git push --force (but check first with git log that this has all relevant commits).

dhevang · 2023-09-18T20:22:02Z

Do you mean to undo the little changes to the files compact/fields/beamline_*.xml?
I meant those to part of this commit but I can undo them if you want. I meant for 19 files to be changed in this PR. All but a few are trivial.

wdconinc · 2023-09-18T20:35:07Z

Please keep the changes to code and changes to quantities separate. Makes it easier to determine if there are any changes introduced by the change to the code that are not due to the changes to the quantities.

dhevang · 2023-09-18T20:49:52Z

Ok, done.

dhevang · 2023-09-20T21:20:31Z

@ajentsch, would this implementation of the BxByBz field map work for your purposes in the B0?

ajentsch · 2023-09-20T22:53:39Z

At first glance, it should. I will be able to test it tomorrow with my field map and give you feedback. Sorry for the delay.

…

On Wed, Sep 20, 2023, 5:20 PM Dhevan Gangadharan ***@***.***> wrote: @ajentsch <https://github.com/ajentsch>, would this implementation of the BxByBz field map work for your purposes in the B0? — Reply to this email directly, view it on GitHub <#527 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADABSF5I3NIWI25BANHUCTLX3NM2VANCNFSM6AAAAAA45HDQVA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

dhevang · 2023-09-21T00:42:40Z

Thanks. you can use the dumpBfield program to print the field in your location of interest, and confirm that the mapping implementation worked.
My PR to extend the functionality of this in DD4hep was merged into their main branch today. Not sure when that propagates to what's in the eic-shell though...

ajentsch

I was able to upload the B0pf field map and see DD4HEP successfully build it. I will do further tests for the B0 map in a separate PR, but the updates to allow bx, by, bz field maps seems to work nicely.

dhevang · 2023-09-21T20:11:56Z

@wdconinc , how often are DD4hep updates propagated to the eic-shell? The new dumpBfield is in their master branch as of yesterday.

wdconinc · 2023-09-21T20:17:50Z

how often are DD4hep updates propagated to the eic-shell?

Normally we update when a new release comes out. I can (and often do) backport patches into eic-shell too (I backported AIDASoft/DD4hep#1161 just yesterday). That often requires that the PR has been merged (so it is stable). Is this about AIDASoft/DD4hep#1170? I can backport that too.

compact/far_backward/lumi/lumi_magnets.xml

src/FieldMapB.cpp

dhevang · 2023-09-21T21:58:32Z

how often are DD4hep updates propagated to the eic-shell?

Normally we update when a new release comes out. I can (and often do) backport patches into eic-shell too (I backported AIDASoft/DD4hep#1161 just yesterday). That often requires that the PR has been merged (so it is stable). Is this about AIDASoft/DD4hep#1170? I can backport that too.

Yes

wdconinc · 2023-09-22T18:40:42Z

DumpBField changes backported and merged into eic-shell. Should get to cvmfs in the next 12 hours.

Attempt to streamline calls to fieldComponents.

Go back to using std::modf, with transient floats.

src/FieldMapB.cpp

Co-authored-by: Dmitry Kalinkin <[email protected]>

…gnet.

Implement lumi dipole B maps.

for more information, see https://pre-commit.ci

Attempt to streamline calls to fieldComponents.

for more information, see https://pre-commit.ci

Multiply by tesla units once when the field is loaded. Remove unnecessary intermediate variable, B_interpolated.

…vector.

Go back to using std::modf, with transient floats.

Co-authored-by: Dmitry Kalinkin <[email protected]>

…nsert_LumiPS_B_FieldMap

…upper limits.

dhevang · 2023-10-12T18:20:21Z

Are there any other optimizations needed for this PR? @ajentsch did some checks using some new field maps in the B0 region, using the new BxByBz option. Things looked good from his perspective.

veprbl

ddsim performance for single electron

main:

        User time (seconds): 8500.01
        System time (seconds): 4.50
        Percent of CPU this job got: 99%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 2:21:45
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 1963884

this branch:

        User time (seconds): 8381.21
        System time (seconds): 4.38
        Percent of CPU this job got: 99%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 2:19:47
        Average shared text size (kbytes): 0
        Average unshared data size (kbytes): 0
        Average stack size (kbytes): 0
        Average total size (kbytes): 0
        Maximum resident set size (kbytes): 1952888

capybara report for reco outputs: https://veprbl.github.io/capybara-reports/f9794437ad70f939c89b642899035e99/

veprbl · 2023-10-17T19:55:45Z

@kkauder feel free to remove from queue if you are going to look into this

dhevang added 3 commits September 18, 2023 13:26

Shift lumi system closer to IP due to proximity with electron CRAB ma…

0e0872d

…gnet.

new B mapping code, which can handle R-Z and X-Y-Z coordinate systems.

31e1e3d

Implement lumi dipole B maps.

Change name of ideal lumi B field values.

ca0b7a0

dhevang requested review from wdconinc and ajentsch September 18, 2023 19:47

dhevang self-assigned this Sep 18, 2023

github-actions bot added topic: far-backward topic: magnets labels Sep 18, 2023

pre-commit-ci bot and others added 2 commits September 18, 2023 19:48

[pre-commit.ci] auto fixes from pre-commit.com hooks

dc3a893

for more information, see https://pre-commit.ci

Merge branch 'main' into Insert_LumiPS_B_FieldMap

b691ccc

Postpone minor changes to another future PR.

bb516a3

wdconinc requested a review from veprbl September 19, 2023 16:19

Merge branch 'main' into Insert_LumiPS_B_FieldMap

1be7ce2

veprbl self-assigned this Sep 20, 2023

ajentsch previously approved these changes Sep 21, 2023

View reviewed changes

wdconinc reviewed Sep 21, 2023

View reviewed changes

compact/far_backward/lumi/lumi_magnets.xml Outdated Show resolved Hide resolved

src/FieldMapB.cpp Outdated Show resolved Hide resolved

Change comment syntax in xml file.

14a2ef4

Attempt to streamline calls to fieldComponents.

dhevang dismissed ajentsch’s stale review via 14a2ef4 September 22, 2023 20:20

dhevang and others added 2 commits October 6, 2023 15:33

Merge branch 'main' into Insert_LumiPS_B_FieldMap

346d405

Bug fix: wrong usage of std::remquof.

c0cafbb

Go back to using std::modf, with transient floats.

veprbl reviewed Oct 9, 2023

View reviewed changes

src/FieldMapB.cpp Outdated Show resolved Hide resolved

dhevang and others added 17 commits October 9, 2023 15:53

Swap Bx and By components in new file as per the recent bug fix.

dba9621

Co-authored-by: Dmitry Kalinkin <[email protected]>

Shift lumi system closer to IP due to proximity with electron CRAB ma…

835566e

…gnet.

new B mapping code, which can handle R-Z and X-Y-Z coordinate systems.

739f103

Implement lumi dipole B maps.

Change name of ideal lumi B field values.

297827a

[pre-commit.ci] auto fixes from pre-commit.com hooks

46360c7

for more information, see https://pre-commit.ci

Postpone minor changes to another future PR.

c8ec64b

Change comment syntax in xml file.

019992c

Attempt to streamline calls to fieldComponents.

Changes as requested in the PR.

ae0783f

[pre-commit.ci] auto fixes from pre-commit.com hooks

5171008

for more information, see https://pre-commit.ci

Remove transformation of the B field for the BrBz case.

069a65c

Multiply by tesla units once when the field is loaded. Remove unnecessary intermediate variable, B_interpolated.

WIP. Add field vector rotation separate from field coord translation …

bb53bbb

…vector.

Bug fix: wrong usage of std::remquof.

eebc4a6

Go back to using std::modf, with transient floats.

Swap Bx and By components in new file as per the recent bug fix.

6903dfd

Co-authored-by: Dmitry Kalinkin <[email protected]>

Merge branch 'Insert_LumiPS_B_FieldMap' of github.com:eic/epic into I…

7991c23

…nsert_LumiPS_B_FieldMap

Merge branch 'main' into Insert_LumiPS_B_FieldMap

81fffa0

Increase grid sizes by 1 in order to prevent seg faults for cases at …

6a2e18c

…upper limits.

Merge branch 'main' into Insert_LumiPS_B_FieldMap

7a2d306

Merge branch 'main' into Insert_LumiPS_B_FieldMap

a412033

veprbl approved these changes Oct 17, 2023

View reviewed changes

veprbl added this pull request to the merge queue Oct 17, 2023

kkauder added topic: geometry topic: simulation labels Oct 17, 2023

Merged via the queue into main with commit 737f84f Oct 17, 2023
85 checks passed

veprbl deleted the Insert_LumiPS_B_FieldMap branch October 17, 2023 21:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement lumi dipole B mapping #527

Implement lumi dipole B mapping #527

dhevang commented Sep 18, 2023

dhevang commented Sep 18, 2023

wdconinc commented Sep 18, 2023

dhevang commented Sep 18, 2023

wdconinc commented Sep 18, 2023

dhevang commented Sep 18, 2023

dhevang commented Sep 20, 2023

ajentsch commented Sep 20, 2023 via email

dhevang commented Sep 21, 2023

ajentsch left a comment

dhevang commented Sep 21, 2023

wdconinc commented Sep 21, 2023

dhevang commented Sep 21, 2023

wdconinc commented Sep 22, 2023

dhevang commented Oct 12, 2023

veprbl left a comment

veprbl commented Oct 17, 2023

Implement lumi dipole B mapping #527

Implement lumi dipole B mapping #527

Conversation

dhevang commented Sep 18, 2023

Briefly, what does this PR introduce?

What kind of change does this PR introduce?

Please check if this PR fulfills the following:

Does this PR introduce breaking changes? What changes might users need to make to their code?

Does this PR change default behavior?

dhevang commented Sep 18, 2023

wdconinc commented Sep 18, 2023

dhevang commented Sep 18, 2023

wdconinc commented Sep 18, 2023

dhevang commented Sep 18, 2023

dhevang commented Sep 20, 2023

ajentsch commented Sep 20, 2023 via email

dhevang commented Sep 21, 2023

ajentsch left a comment

Choose a reason for hiding this comment

dhevang commented Sep 21, 2023

wdconinc commented Sep 21, 2023

dhevang commented Sep 21, 2023

wdconinc commented Sep 22, 2023

dhevang commented Oct 12, 2023

veprbl left a comment

Choose a reason for hiding this comment

veprbl commented Oct 17, 2023