-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Alpaka magnetic field polish #48729
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
Merged
cmsbuild
merged 4 commits into
cms-sw:master
from
alexstrel:Alpaka-magnetic-field-polish
Nov 13, 2025
Merged
Alpaka magnetic field polish #48729
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
47 changes: 0 additions & 47 deletions
47
MagneticField/ParametrizedEngine/interface/ParabolicParametrizedMagneticField.h
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| <use name="xtd"/> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
|
|
||
| # MagneticField/Portable | ||
|
|
||
| This package will host simple and efficient implementations of the magnetic field map for use in well-defined scenarios on GPUs and other heterogeneous hardware. | ||
|
|
||
| ## ParabolicMagneticField | ||
|
|
||
| The parabolic parametrisation of the magnetic field was optimised for extrapolation within R < 115 cm and |Z| < 280 cm, which is (almost) up to the surface of ECAL. | ||
| Beyond that it will diverge from the actual magnetic field map, especially at large eta, in ways that are hard to quantify. | ||
|
|
||
| This is based on a simple parabolic formula of Bz(z), neglecting the Br and Bphi components, and is thus vert fast. To account for the missing Br component, which contributes to transverse bending, the parametrization is tuned to reproduce as closely as possible the field integral computed over a straight path originated in (0,0,0) and across the whole tracker, rather than the value of Bz. More details can be found [here](https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideMagneticField#Alternative_parametrizations_of). | ||
|
|
||
|
|
||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| #ifndef MagneticField_Portable_interface_ParabolicMagneticField_h | ||
| #define MagneticField_Portable_interface_ParabolicMagneticField_h | ||
|
|
||
| #include <xtd/stdlib/abs.h> | ||
|
|
||
| namespace portableParabolicMagneticField { | ||
|
|
||
| struct Parameters { | ||
| // These parameters are the best fit of 3.8T to the OAEParametrizedMagneticField parametrization. | ||
|
|
||
| static constexpr float c1 = 3.8114; | ||
| static constexpr float b0 = -3.94991e-06; | ||
| static constexpr float b1 = 7.53701e-06; | ||
| static constexpr float a = 2.43878e-11; | ||
| static constexpr float max_radius2 = 13225.f; // tracker radius | ||
| static constexpr float max_z = 280.f; // tracker z | ||
| }; | ||
|
|
||
| template <typename Vec3> | ||
| constexpr float Kr(Vec3 const& vec) { | ||
| return Parameters::a * (vec[0] * vec[0] + vec[1] * vec[1]) + 1.f; | ||
| } | ||
|
|
||
| template <typename Vec3> | ||
| constexpr float B0Z(Vec3 const& vec) { | ||
| return Parameters::b0 * vec[2] * vec[2] + Parameters::b1 * vec[2] + Parameters::c1; | ||
| } | ||
|
|
||
| template <typename Vec3> | ||
| constexpr inline bool isValid(Vec3 const& vec) { | ||
| return ((vec[0] * vec[0] + vec[1] * vec[1]) < Parameters::max_radius2 && xtd::abs(vec[2]) < Parameters::max_z); | ||
| } | ||
|
|
||
| template <typename Vec3> | ||
| constexpr inline float magneticFieldAtPoint(Vec3 const& vec) { | ||
| if (isValid(vec)) { | ||
| return B0Z(vec) * Kr(vec); | ||
| } else { | ||
| return 0; | ||
| } | ||
| } | ||
|
|
||
| } // namespace portableParabolicMagneticField | ||
| #endif |
4 changes: 2 additions & 2 deletions
4
...eld/ParametrizedEngine/test/BuildFile.xml → MagneticField/Portable/test/BuildFile.xml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,9 @@ | ||
| <bin file="alpaka/testParabolicParametrizedMagneticField.dev.cc"> | ||
| <bin file="alpaka/testParabolicParametrizedTrackerField.dev.cc"> | ||
| <use name="alpaka"/> | ||
| <use name="eigen"/> | ||
| <use name="DataFormats/GeometryVector"/> | ||
| <use name="FWCore/Utilities"/> | ||
| <use name="HeterogeneousCore/AlpakaInterface"/> | ||
| <use name="MagneticField/ParametrizedEngine" source_only="1"/> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line should be kept, but as <use name="MagneticField/Portable"/> |
||
| <use name="MagneticField/Portable"/> | ||
| <flags ALPAKA_BACKENDS="1"/> | ||
| </bin> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the name
PortableParametrizedTrackerFieldfor the new package is misleading.Based on the expected use case, isn't this going to be used at least up to ECAL ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, also the existing parametric magnetic field is not called "Tracker".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a new package is the way to go, I would suggest to call it just
MagneticField/Portableor something along that line.Then we can add more options (the uniform MF, if anybody bothers with it instead of hard-coding the central value; and anything more precise we may come up with).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This specific parametrization was optimized for extrapolation within R<115, |Z|<280 cm, which is indeed (almost) up to the surface of ECAL. But beyond that, it will diverge from the real map, especially at large eta, although I don't have numbers to quantify how much at hand.
Note that the original
ParabolicParametrizedTrackerField, If called outside that range, it returns (0,0,0) and issues a LogDebug message. TheisDefinedmethod is used, in particular, to allow overlaying parametrizations of the inner region over the full CMS map, which is based on grid interpolation.Indeed
MagneticField/Portableleaves more flexibility for any future development one could possibly imagine. On the other hand, it will trick an unaware user to think it's the way to go for any application, i.e. to extrapolate to muon stations, where it will give nonsense.MagneticField/PortableParametrizationsis possibly slightly better (somehow suggests that it provides only parametrizations of the real thing); nothing better comes to my mind at the moment.PS: we already have a uniform MF option. cf. uniformMagneticField_cfi.py, that has notably been used to process cosmic data with B off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know that we have a uniform magnetic field in CMSSW, I meant for adding a "portable" one, like this parametric implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name
ParabolicParametrizedFieldhas been used for the past 12 years. I don't think it "tricked" people then, and I don't think people were particularly wiser than today.If there is a strong-enough reason to adopt a new name, the same change should be applied consistently to the alpaka and non-alpaka versions, but that is well beyond the scope of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I pointed out a few times already, MagneticField/ParametrizedEngine/src/ParabolicParametrizedMagneticField.h is part of the private implementation of the package, which was done exactly to prevent it being used directly.
We are now creating a new version that is exposed in the public interface, so we are giving direct access to people now, which has not been the case in the past 12 years.
In any case, it's not a matter of whose name is the longest. Having two files in different places with the same name is a not particularly wise choice, for reasons that should be obvious. If they are different, they should be called differently.
So, please avoid calling the new class with the same name of an already existing class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it would be better to use different names for completely different concepts.
Yes, I think it makes a lot of sense to use the same name for the same concept in different contexts.
See
edm::EDProducervsedm::one::EDProducervsedm::stream::EDProducervsedm::global::EDProducervsALPAKA_ACCELERATOR_NAMESPACE::stream::EDProducervsALPAKA_ACCELERATOR_NAMESPACE::global::EDProducer:Or
edm::Eventvsfwlite::EventvsALPAKA_ACCELERATOR_NAMESPACE::device::Event:And so on.
Anyway, since you feel so strongly that you own the name
ParabolicParametrizedField, let's pick a new subsystem, package and name for this.