Skip to content

feat: use BoundedMultipoleMagnet with AABB pre-filter to MultipoleMagnet#1048

Open
wdconinc wants to merge 2 commits intomainfrom
bounded-multipole-magnet
Open

feat: use BoundedMultipoleMagnet with AABB pre-filter to MultipoleMagnet#1048
wdconinc wants to merge 2 commits intomainfrom
bounded-multipole-magnet

Conversation

@wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Feb 25, 2026

Briefly, what does this PR introduce?

This PR adds a pre-filter to the MultipoleMagnet to apply an axis-aligned bounding box. This avoids expensive rotated r-z Tube::Contains evaluation in many cases, in particular since our fields are all added in an OverlayField, and assigned to the global field manager (i.e. evaluate for the far forward and far backward regions for every particle in the EM calorimeter showers).

Profiling indicates we spend 30% of the total ddsim (no optical photons) simulation time (for DIS 18x275 Q2>1000) in dd4hep::MultipoleField::fieldComponents, of which 25% (of the total simulation time) in the Tube::Contains (this is based on callgrind). This PR aims to address that (more profiling below).

Other options considered:

  • assigning the multipole fields to a geometry volume so we only need to evaluate them when inside those: discarded since it doesn't handle the stray fields in the gaps between magnets well (and I think @ajentsch once said that's important).
  • placing the entire far forward and far backward regions inside their own volume (not assembly as currently): discarded since this would require changes to the geometry structure and a clean interface between inside/outside these volumes, which is hard to do without clear axis-aligned boundaries.

Comparing profiling in the ctree artifacts (for DIS 10x100 Q2 > 1000 this time, sorry), we see the total time of simulation go from 822s to 768s. The time in dd4hep::MultipoleField::fieldComponents (in the various places in the call tree where it is called) goes from 45+5+6+4s to 12+1+1+1s, for a 6% speed increase.

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue: MultipoleMagnet is performance hog)
  • 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?

No.

Does this PR change default behavior?

No.

Copilot AI review requested due to automatic review settings February 25, 2026 20:03
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a performance optimization for multipole magnet field evaluations by adding an axis-aligned bounding box (AABB) pre-filter to the MultipoleMagnet field type. The new BoundedMultipoleMagnet is designed as a drop-in replacement that performs a cheap AABB check before delegating to the expensive rotated tube containment check and multipole field calculation. According to the PR description, this addresses a significant performance bottleneck where 30% of simulation time was spent in field evaluations, with 25% of that in Tube::Contains checks.

Changes:

  • New BoundedMultipoleMagnet.cpp implementation with AABB pre-filtering
  • Updated all far-forward and far-backward multipole magnet field definitions to use the new field type
  • Updated electron beamline quadrupole field definitions

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/BoundedMultipoleMagnet.cpp New field plugin that adds AABB pre-filter to DD4hep's MultipoleField
compact/far_forward/magnets.xml Updated 8 magnet fields to use BoundedMultipoleMagnet
compact/far_forward/electron_beamline.xml Updated 2 quadrupole fields to use BoundedMultipoleMagnet
compact/far_backward/magnets.xml Updated 5 magnet fields to use BoundedMultipoleMagnet
compact/far_backward/lumi/lumi_magnets.xml Updated 2 commented-out ideal field definitions to use BoundedMultipoleMagnet

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants