RungeKuttaUtils: Add comments on what exactly we do

This merge request affects 1 package:

• Tracking/TrkExtrapolation/TrkExUtils

Adding @amorley ,@gavrilen as watchers

added Tracking master review-pending-level-1 labels

Since it was ask in the original one

vbroadcast is vec2 V1 = V[0] - vec2{0}.

The reason (mnemonic) I prefer to call this / use broadcast is because it fits with nomeclature of what trying to do (broadcast) foo4 https://godbolt.org/z/9zxYar

CI Result SUCCESS (hash b631c28f)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21552]

added review-pending-level-2 label and removed review-pending-level-1 label

Asking for an L2 shifter to look at L157-L169. I'm struggling with the mathematics of this change, and while I'm pretty sure the conversion from Vec2 to Vec4 is fine, I'd like a second pair of eyes to just check.

Hi,

   Jac[ 0] =  V[0]*P[ 0]+V[1]*P[ 1]+V[2]*P[ 2];                               // dL0/dL0
    Jac[ 1] = V[0]*P[7]+ V[1]*P[8]+ V[2]*P[9];                               // dL0/dL1
    Jac[ 2] = V[0]*P[14]+V[1]*P[15]+V[2]*P[16];                               // dL0/dPhi
    Jac[ 3] = V[0]*P[21]+V[1]*P[22]+V[2]*P[23];                               // dL0/dThe

is the original

  vec4 Pv1 = { P[0], P[7], P[14], P[21] };
  vec4 Pv2 = { P[1], P[8], P[15], P[22] };
  vec4 Pv3 = { P[2], P[9], P[16], P[23] };
  // 1st to 4th element
  vec4 res = V1 * Pv1 + V2 * Pv2 + V3 * Pv3;

is the same. The tricky bit is that the vec4, are to be read vertical . e.g

vec4 res = V1 * Pv1 + V2 * Pv2 + V3 * Pv3;

is 4 lines The {} below are kind of vertical

*We want to do
   * {Jac[0],Jac[1],Jac[2],Jac[3]} =
   * {V[0],V[0],V[0],V[0]} * {P[0],P[7],P[14],P[21]} +
   * {V[1],V[1],V[1],V[1]} * {P[1],P[8],P[15],P[22]} +
   * {V[2],V[2],V[2],V[2]} * {P[2],P[9],P[16],P[23}

Maybe I can try to format it better ?

added 1 commit

• 7d10a8d5 - update comments

Compare with previous version

This merge request affects 1 package:

• Tracking/TrkExtrapolation/TrkExUtils

Adding @amorley ,@gavrilen as watchers

added review-pending-level-1 label and removed review-pending-level-2 label

So @nreadiof I added some extra comments. The kind of calculation as was it was already mentioned to beginning of the function. Now the comment inside reads.

  /*We want first to do
   *
   * |Jac[0] | |V[0]|  |P[0] | |V[1]|  |P[1] |  |V[2]|  |P[2] |
   * |Jac[1] |=|V[0]| *|P[7] |+|V[1]|* |P[8] | +|V[2]| *|P[9] |
   * |Jac[2] | |V[0]|  |P[14]| |V[1]|  |P[15]|  |V[2]|  |P[16]|
   * |Jac[3] | |V[0]|  |P[21]| |V[1]|  |P[22]|  |V[2]|  |P[23]|
   *
   * where its veertical column goes to a SIMD vector of size 4.
   * The P indices come from the natoation of the P array use
   * in this code.
   * Look comment on the function.
   *
   * so we end up with
   * {Jac[0],Jac[1],Jac[2],Jac[3]} =
   * {V[0],V[0],V[0],V[0]} * {P[0],P[7],P[14],P[21]} +
   * {V[1],V[1],V[1],V[1]} * {P[1],P[8],P[15],P[22]} +
   * {V[2],V[2],V[2],V[2]} * {P[2],P[9],P[16],P[23}
   *
   * Where  {} is a SIMD size 4 vector
   *
   * The remaining odd element is done at the end
   * Jac[4] = V[0] * P[28] + V[1] * P[29] + V[2] * P[30];

The "original" kind of way was

   * Jac[ 0] = Ax[0]*P[ 7]+Ax[1]*P[ 8]+Ax[2]*P[ 9];                               // dL0/dL0
   * Jac[ 1] = Ax[0]*P[14]+Ax[1]*P[15]+Ax[2]*P[16];                               // dL0/dL1
   * Jac[ 2] = Ax[0]*P[21]+Ax[1]*P[22]+Ax[2]*P[23];                               // dL0/dPhi
   * Jac[ 3] = Ax[0]*P[28]+Ax[1]*P[29]+Ax[2]*P[30];                               // dL0/dThe
   * Jac[ 4] = Ax[0]*P[35]+Ax[1]*P[36]+Ax[2]*P[37];                               // dL0/dCM

There is is 1*3 \times 3x5 giving 1x5. The only tricky part is the way the P is used in this package, there is a stride of 7 between rows of the 3x5 but this is not "new".

added 1 commit

• 49ee1852 - fix typo in comment

Compare with previous version

This merge request affects 1 package:

• Tracking/TrkExtrapolation/TrkExUtils

Adding @amorley ,@gavrilen as watchers

CI Result SUCCESS (hash 7d10a8d5)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21653]

CI Result SUCCESS (hash 49ee1852)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21654]

Hi @christos,

sorry, I'm still confused. How do I get e.g.

Jac[ 0] = Ax[0]*P[ 7]+Ax[1]*P[ 8]+Ax[2]*P[ 9]; // dL0/dL0

from

{Jac[0],Jac[1],Jac[2],Jac[3]} =
   {V[0],V[0],V[0],V[0]} * {P[0],P[7],P[14],P[21]} +
   {V[1],V[1],V[1],V[1]} * {P[1],P[8],P[15],P[22]} +
   {V[2],V[2],V[2],V[2]} * {P[2],P[9],P[16],P[23}

Requesting L2 to have a look as well.

Cheers, Volker (L1)

added review-pending-level-2 label and removed review-pending-level-1 label

OK but I am interested on what is not clear

The one example what you ask for is

 * Jac[ 0] = Ax[0]*P[ 7]+Ax[1]*P[ 8]+Ax[2]*P[ 9]; // dL0/dL0
 * Jac[ 1] = Ax[0]*P[14]+Ax[1]*P[15]+Ax[2]*P[16];  // dL0/dL1
 * Jac[ 2] = Ax[0]*P[21]+Ax[1]*P[22]+Ax[2]*P[23]; // dL0/dPhi
 * Jac[ 3] = Ax[0]*P[28]+Ax[1]*P[29]+Ax[2]*P[30]; // dL0/dThe
 * Jac[ 4] = Ax[0]*P[35]+Ax[1]*P[36]+Ax[2]*P[37]; // dL0/dCM
 * Jac[ 5] = Ay[0]*P[ 7]+Ay[1]*P[ 8]+Ay[2]*P[ 9]; // dL1/dL0
 * Jac[ 6] = Ay[0]*P[14]+Ay[1]*P[15]+Ay[2]*P[16]; // dL1/dL1
 * Jac[ 7] = Ay[0]*P[21]+Ay[1]*P[22]+Ay[2]*P[23];  // dL1/dPhi
 * Jac[ 8] = Ay[0]*P[28]+Ay[1]*P[29]+Ay[2]*P[30]; // dL1/dThe
 * Jac[ 9] = Ay[0]*P[35]+Ay[1]*P[36]+Ay[2]*P[37]; // dL1/dCM

right? which was done , pre-existing code here.

 mutl3x5Helper(&Jac[0],Ax,&P[7]);
  mutl3x5Helper(&Jac[5],Ay,&P[7]);

correct ?

So is it the stide that is confusing ? The fact that there is a stride over 7 from starting point in the array that is not clear?

or the thing I added new comment on

/*We want first to do
   *
   * |Jac[0] | |V[0]|  |P[0] | |V[1]|  |P[1] |  |V[2]|  |P[2] |
   * |Jac[1] |=|V[0]| *|P[7] |+|V[1]|* |P[8] | +|V[2]| *|P[9] |
   * |Jac[2] | |V[0]|  |P[14]| |V[1]|  |P[15]|  |V[2]|  |P[16]|
   * |Jac[3] | |V[0]|  |P[21]| |V[1]|  |P[22]|  |V[2]|  |P[23]|
   *
   * where its veertical column goes to a SIMD vector of size 4.
   * The P indices come from the natoation of the P array used
   * in this code.
   * Look comment on the function.
   *
   * so we end up with
   * {Jac[0],Jac[1],Jac[2],Jac[3]} =
   * {V[0],V[0],V[0],V[0]} * {P[0],P[7],P[14],P[21]} +
   * {V[1],V[1],V[1],V[1]} * {P[1],P[8],P[15],P[22]} +
   * {V[2],V[2],V[2],V[2]} * {P[2],P[9],P[16],P[23}
   *
   * Where  {} is a SIMD size 4 vector
   *
   * The remaining odd element is done at the end
   * Jac[4] = V[0] * P[28] + V[1] * P[29] + V[2] * P[30];

What we do in math is the 1st , what you see in simd is the 2nd . the issue is that SIMD although in code look like {} are "vertical" in nature this why I added both.

Basically, you start counting from 7 since is the 0th of the passed array, and you rotate 90 degrees or so to get what is above in the comment.

added 1 commit

• 4b985b97 - Let's leave the code (as it the same) and leave the comments (which seem to bring the confusion?)

Compare with previous version

changed title from RungeKuttaUtils: mutl3x5Helper Simplify and add a bit more comments to RungeKuttaUtils: Add comments on what exactly we do

This merge request affects 1 package:

• Tracking/TrkExtrapolation/TrkExUtils

Adding @amorley ,@gavrilen as watchers

added review-pending-level-1 label and removed review-pending-level-2 label

added 1 commit

• 3ce463bb - Let's leave the code (as it the same) and leave the comments (which seem to bring the confusion?)

Compare with previous version

This merge request affects 1 package:

• Tracking/TrkExtrapolation/TrkExUtils

Adding @amorley ,@gavrilen as watchers

added 1 commit

• b7e25571 - Add even more comments on how the mutl3x5Helper is to be read

Compare with previous version

This merge request affects 1 package:

• Tracking/TrkExtrapolation/TrkExUtils

Adding @amorley ,@gavrilen as watchers

CI Result SUCCESS (hash 3ce463bb)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21739]

CI Result SUCCESS (hash 4b985b97)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21738]

CI Result SUCCESS (hash b7e25571)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21740]

added review-approved label and removed review-pending-level-1 label

This has merge conflicts now I'm afraid! Sorry!

added review-user-action-required label and removed review-approved label

added 527 commits

• b7e25571...28d2e8d2 - 526 commits from branch atlas:master
• 25c3ea18 - Merge branch 'master' into 'RkUtils_3x5Helper'

Compare with previous version

This merge request affects 1 package:

• Tracking/TrkExtrapolation/TrkExUtils

Adding @amorley ,@gavrilen as watchers

added review-pending-level-1 label and removed review-user-action-required label

OK the auto merge resolution seems to have back fired will do a manual one

added 1 commit

• 844cd49f - Proper conflict resolution

Compare with previous version

This merge request affects 1 package:

• Tracking/TrkExtrapolation/TrkExUtils

Adding @amorley ,@gavrilen as watchers

CI Result FAILURE (hash 25c3ea18)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 1, warnings 9
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21842]

CI Result SUCCESS (hash 844cd49f)

	Athena	AthSimulation	AthGeneration	AnalysisBase
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 21844]

added review-approved label and removed review-pending-level-1 label

merged

mentioned in commit 753588ad

added sweep:ignore label