Skip to content
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

Fix true position/momentum at z #134

Merged
merged 2 commits into from
Aug 30, 2024
Merged

Fix true position/momentum at z #134

merged 2 commits into from
Aug 30, 2024

Conversation

jdkio
Copy link
Contributor

@jdkio jdkio commented Aug 27, 2024

Asa found that the true position at the start of the TMS and position at the start of the track are wrong. Here green are start positions while black are true hits.
image

Turns out the GetPositionAtZ funciton wasn't working well, and neither was GetMomentumAtZ. It seems like it's returning the start position each time, and adjusting z to match the requested z. That's why the black line is pointing to x and y in the x vs y plot.

Here are some x,y,z,t times for BirthPosition, RecoTrackPrimaryParticleTruePositionStart, PositionTMSStart, RecoTrackPrimaryParticleTruePositionTrackStart, RecoTrackPrimaryParticleTruePositionTrackEnd and RecoTrackPrimaryParticleTruePositionEnd.

Truth_Info->Scan("BirthPosition[RecoTrackPrimaryParticleIndex]:RecoTrackPrimaryParticleTruePositionStart:PositionTMSStart[RecoTrackPrimaryParticleIndex]:RecoTrackPrimaryParticleTruePositionTrackStart:RecoTrackPrimaryParticleTruePositionTrackEnd:RecoTrackPrimaryParticleTruePositionEnd", "RecoTrackN>0")
*        1 *        0 * -1603.209 * -1603.209 * -250.6051 * -1603.209 * -1603.209 * 314.20971 *
*        1 *        1 * -932.6213 * -932.6213 * -457.7382 * -932.6213 * -932.6213 * -58.97908 *
*        1 *        2 * 6885.9316 * 6885.9316 *     11538 *     11423 *     15513 * 15488.617 *
*        1 *        3 *         1 *         1 * 17.252395 *         1 *         1 * 31.016029 *
*        3 *        0 * -2062.861 * -2062.861 * 435.09262 * -2062.861 * -2062.861 * 882.00091 *
*        3 *        1 * -2341.010 * -2341.010 * -2594.115 * -2341.010 * -2341.010 * -2675.035 *
*        3 *        2 *  4816.646 *  4816.646 *     11403 *     11423 *     12468 * 12465.306 *
*        3 *        3 *         1 *         1 * 24.763738 *         1 *         1 * 29.141386 *
*       14 *        0 * 1321.6666 * 1321.6666 * 1367.8759 * 1321.6666 * 1321.6666 * -2057.476 *
*       14 *        1 * -2049.869 * -2049.869 * -2222.441 * -2049.869 * -2049.869 * -4086.726 *
*       14 *        2 * 9104.9462 * 9104.9462 * 11516.502 *     11423 *     18313 * 23997.279 *
*       14 *        3 *         1 *         1 * 9.0671720 *         1 *         1 * 54.360744 *

Of these, RecoTrackPrimaryParticleTruePositionStart, RecoTrackPrimaryParticleTruePositionTrackStart, RecoTrackPrimaryParticleTruePositionTrackEnd all use GetPositionAtZ. We can see they're all the birth position except that z was adjusted to match the requested z position. RecoTrackPrimaryParticleTruePositionEnd uses GetDeathPosition, and PositionTMSStart uses GetPositionEnteringTMS.

Fix

I replaced GetPositionAtZ and GetMomentumAtZ with an improved version. This improved version does a linear interpolation between the two nearest points to try to get the most exact value for a given value of z. If z is outside the range of the particle, but within max_z_dist, then it returns the start/end position/momentum. This was always on my todo list but given this bug, I went ahead and did it

This also fixes the issue:

Truth_Info->Scan("BirthPosition[RecoTrackPrimaryParticleIndex]:RecoTrackPrimaryParticleTruePositionStart:PositionTMSStart[RecoTrackPrimaryParticleIndex]:RecoTrackPrimaryParticleTruePositionTrackStart:RecoTrackPrimaryParticleTruePositionTrackEnd:RecoTrackPrimaryParticleTruePositionEnd", "RecoTrackN>0")
*        1 *        0 * -1603.209 * -1603.209 * -250.6051 * -284.1145 * 314.20971 * 314.20971 *
*        1 *        1 * -932.6213 * -932.6213 * -457.7382 * -469.5696 * -58.97908 * -58.97908 *
*        1 *        2 * 6885.9316 * 6885.9316 *     11538 *     11423 * 15488.617 * 15488.617 *
*        1 *        3 *         1 *         1 * 17.252395 * 16.850486 * 31.016029 * 31.016029 *
*        3 *        0 * -2062.861 * -2062.861 * 435.09262 * 442.10052 * 882.00091 * 882.00091 *
*        3 *        1 * -2341.010 * -2341.010 * -2594.115 * -2594.989 * -2675.035 * -2675.035 *
*        3 *        2 *  4816.646 *  4816.646 *     11403 *     11423 * 12465.306 * 12465.306 *
*        3 *        3 *         1 *         1 * 24.763738 * 24.835876 * 29.141386 * 29.141386 *
*       14 *        0 * 1321.6666 * 1321.6666 * 1367.8759 * 1366.0842 * 646.04101 * -2057.476 *
*       14 *        1 * -2049.869 * -2049.869 * -2222.441 * -2215.750 * -2773.358 * -4086.726 *
*       14 *        2 * 9104.9462 * 9104.9462 * 11516.502 *     11423 *     18313 * 23997.279 *
*       14 *        3 *         1 *         1 * 9.0671720 * 8.7543878 * 32.094665 * 54.360744 *
*       16 *        0 * -3373.777 * -3373.777 * -2986.629 * -2991.291 * -2986.772 * -2992.386 *
*       16 *        1 * -146.9955 * -146.9955 * -1561.424 * -1546.192 * -2480.199 * -2496.546 *
*       16 *        2 * 6859.4209 * 6859.4209 *     11473 *     11423 *     14393 * 14435.225 *
*       16 *        3 *         1 *         1 * 17.175430 * 17.000026 * 27.604330 * 749.89959 *

I made some test files using the validation technique . I tagged with kleykamp_2024-08-27 and stored them in /exp/dune/data/users/kleykamp/dune-tms/simple_processing_test/kleykamp_2024-08-27

@jdkio jdkio requested review from LiamOS and AsaNehm August 27, 2024 20:32
@jdkio jdkio added the bug Something isn't working label Aug 27, 2024
Copy link
Contributor

@AsaNehm AsaNehm left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Still need to actually test it out though

@AsaNehm
Copy link
Contributor

AsaNehm commented Aug 28, 2024

spill_000
Seems good to me

Copy link
Member

@LiamOS LiamOS left a comment

Choose a reason for hiding this comment

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

👍 Good fix.

@jdkio jdkio merged commit a41e7ee into main Aug 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants