Alpaka magnetic field polish #48729
Alpaka magnetic field polish #48729
Conversation
|
cms-bot internal usage |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48729/45780 Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
cmsbuild
added
code-checks-rejected
code-checks-pending
and removed
code-checks-pending
code-checks-rejected
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48729/45781 |
|
A new Pull Request was created by @alexstrel for master. It involves the following packages:
@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
assign heterogeneous |
| #ifndef MagneticField_ParametrizedEngine_interface_ParabolicParametrizedMagneticField_h | ||
| #define MagneticField_ParametrizedEngine_interface_ParabolicParametrizedMagneticField_h | ||
|
|
||
| #include <Eigen/Core> |
There was a problem hiding this comment.
This #include should not be needed, because this header does not directly depend on Eigen.
| #include <Eigen/Core> |
| template <typename Vec3> | ||
| constexpr float Kr(Vec3 const& vec) { | ||
| return Parameters::a * (vec(0) * vec(0) + vec(1) * vec(1)) + 1.f; | ||
| return Parameters::a * (vec[0] * vec[0] + vec[1] * vec[1]) + 1.f; |
There was a problem hiding this comment.
I guess the operator[] works also with Eigen::Matrix<float, 3, 1> (that the unit test uses). Arguably for 1D vector the operator[] is more general as works e.g. with C-array and std::array.
There was a problem hiding this comment.
yes, that is more generic form.
| template <typename TAcc, typename T> | ||
| ALPAKA_FN_ACC void operator()(TAcc const& acc, T const* __restrict__ in, T* __restrict__ out, size_t size) const { |
There was a problem hiding this comment.
Please keep this as
| template <typename TAcc, typename T> | |
| ALPAKA_FN_ACC void operator()(TAcc const& acc, T const* __restrict__ in, T* __restrict__ out, size_t size) const { | |
| template <typename T> | |
| ALPAKA_FN_ACC void operator()(Acc1D const& acc, T const* __restrict__ in, T* __restrict__ out, size_t size) const { |
|
|
||
| float resolution = 0.2; | ||
| const float resolution = 0.2; | ||
| // |
There was a problem hiding this comment.
Is this comment-only line needed?
| // Hard-wired numbers defining the surfaces on which the crystal front faces lie | ||
|
|
There was a problem hiding this comment.
I wonder where this line originates from. Maybe an earlier version of #44360? I'd remove these two lines from this PR (unless there is a strong reason to keep)
| // Hard-wired numbers defining the surfaces on which the crystal front faces lie |
There was a problem hiding this comment.
This is an artifact from Charis's code, removed
|
enable gpu |
|
@cmsbuild, please test |
|
+1 Size: This PR adds an extra 16KB to repository Comparison SummarySummary:
CUDA Comparison SummarySummary:
ROCM Comparison SummarySummary:
|
cmsbuild
added
tests-approved
tests-pending
and removed
tests-started
tests-approved
code-checks-approved
labels
cmsbuild
added
tests-pending
code-checks-pending
and removed
tests-approved
code-checks-approved
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48729/46810 |
|
@cmsbuild, please test |
|
Thanks @alexstrel. Last request, could you squash all commits into one ? |
|
+heterogeneous |
|
+1 Size: This PR adds an extra 20KB to repository Comparison SummarySummary:
AMD_MI300X Comparison SummarySummary:
AMD_W7900 Comparison SummarySummary:
NVIDIA_H100 Comparison SummarySummary:
NVIDIA_L40S Comparison SummarySummary:
NVIDIA_T4 Comparison SummarySummary:
|
|
+1 |
cmsbuild
added
reconstruction-approved
fully-signed
and removed
reconstruction-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @ftenchini, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
7 participants
PR description:
This PR adopts ParabolicParametrizedMagneticField.h parameter structure and helper functions for Alpaka implementations. Will be used in Alpaka electron seeding code. Unit test is modified to take into account the proposed changes.
resolves cms-sw/framework-team#1517 (comment)