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

Add pT Runtime Toggle + pT Toggle for Geometry Inputs #39

Conversation

GNiendorf
Copy link
Member

@GNiendorf GNiendorf commented Jun 10, 2024

This PR moves the compile-time pT flag to a runtime pT option and adds an option to toggle between the low pT and normal module maps. The most significant change with this PR is that it removes the pT cut from the global variables and instead passes it to each of the relevant kernels.

@GNiendorf
Copy link
Member Author

/run all

Copy link

There was a problem while building and running in standalone mode. The logs can be found here.

Copy link

There was a problem while building and running with CMSSW. The logs can be found here.

@GNiendorf
Copy link
Member Author

@VourMa Are the errors I get from the standalone check fixed in your PR's? It says something about Event.h not being found.

@GNiendorf GNiendorf force-pushed the CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles_TrackLooperPR409 branch from 69c5b81 to 6873c97 Compare June 10, 2024 20:51
@GNiendorf
Copy link
Member Author

/run all

Copy link

There was a problem while building and running in standalone mode. The logs can be found here.

Copy link

There was a problem while building and running with CMSSW. The logs can be found here.

@VourMa
Copy link
Collaborator

VourMa commented Jun 10, 2024

@VourMa Are the errors I get from the standalone check fixed in your PR's? It says something about Event.h not being found.

CI worked for #37 (which didn't include the "fixes", which apply only on local validation), so it should be something else, I guess.

@GNiendorf GNiendorf force-pushed the CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles_TrackLooperPR409 branch from 3af2e0d to 2a387f8 Compare June 10, 2024 21:34
@GNiendorf
Copy link
Member Author

/run all

@SegmentLinking SegmentLinking deleted a comment from github-actions bot Jun 10, 2024
Copy link

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     44.1    326.4    122.8     47.8     95.1    506.4    132.8    158.2    100.5      2.6    1536.7     986.2+/- 264.2     417.5   explicit_cache[s=4] (target branch)
   avg     47.3    322.3    147.4     48.3     96.0    506.6    124.3    178.4     98.1      2.0    1570.5    1016.6+/- 278.2     429.3   explicit_cache[s=4] (this PR)

@VourMa
Copy link
Collaborator

VourMa commented Jun 10, 2024

There was a problem while building and running with CMSSW. The logs can be found here.

It seems like the reason why the CMSSW part failed is because #37 was merged and now there are conflicts that the CI doesn't know how to resolve. @GNiendorf could you resolve them and run the test again?

… into CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles_TrackLooperPR409
@GNiendorf
Copy link
Member Author

/run all

Copy link

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     47.7    325.8    123.0     48.5     96.2    498.6    133.7    157.4    103.6      2.2    1536.8     990.4+/- 264.8     416.7   explicit_cache[s=4] (target branch)
   avg     43.2    323.9    150.7     49.3     97.4    545.8    124.6    179.6    109.7      2.4    1626.7    1037.6+/- 281.4     447.5   explicit_cache[s=4] (this PR)

Copy link

There was a problem while building and running with CMSSW. The logs can be found here.

@VourMa
Copy link
Collaborator

VourMa commented Jun 11, 2024

@GNiendorf Could you try compiling locally before running the tests, so that you can debug at your convenience? If you get stuck, please make a comment with the error and I can take a look tomorrow, and provide instructions or fix it.

@GNiendorf
Copy link
Member Author

@GNiendorf Could you try compiling locally before running the tests, so that you can debug at your convenience? If you get stuck, please make a comment with the error and I can take a look tomorrow, and provide instructions or fix it.

Yup of course, you asked me to fix the conflicts and rerun the tests so that's what I did above, but I wanted to spend more time on the CMSSW error myself before posting the error and asking for help. My main concern was getting the standalone tests working for today.

@GNiendorf
Copy link
Member Author

/run all

Copy link

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     48.2    325.5    122.9     49.2     96.9    493.0    131.9    157.8    105.4      2.2    1533.1     991.9+/- 265.1     420.4   explicit_cache[s=4] (target branch)
   avg     43.6    327.1    148.8     49.2     97.3    547.9    125.5    179.2    107.8      2.2    1628.5    1037.1+/- 274.9     440.7   explicit_cache[s=4] (this PR)

Copy link

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

@GNiendorf
Copy link
Member Author

/run all

Copy link

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     44.7    324.6    124.4     51.1     97.8    502.0    119.7    162.6    105.2      3.6    1535.7     989.0+/- 263.2     424.9   explicit_cache[s=4] (target branch)
   avg     47.9    325.2    121.3     50.3     99.0    507.5    121.3    161.4    105.5      3.7    1543.0     987.6+/- 254.6     423.4   explicit_cache[s=4] (this PR)

@GNiendorf
Copy link
Member Author

/run all

Copy link

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     44.5    325.7    124.3     50.4     98.0    505.2    120.5    164.7    106.2      2.9    1542.4     992.7+/- 263.4     423.0   explicit_cache[s=4] (target branch)
   avg     47.7    324.7    123.1     50.2    100.4    517.1    121.2    164.6    107.2      1.8    1558.0     993.2+/- 262.5     425.6   explicit_cache[s=4] (this PR)

Copy link

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

@GNiendorf
Copy link
Member Author

@GNiendorf The changes look good to me. Just writing down the list of things to do to move forward with merging:

  • Provide physics performance plots with more stats and follow up depending on the results.
  • Deal with double-to-float conversion one way or the other.
  • Move the error check for pT < 0.6 GeV to a more appropriate place and throw the error in the appropriate way.

Instead of addressing the first bullet point in this PR I'm adding a flag to switch between the two maps like we discussed a week or so ago. The solution I added in the last few commits works, but I'm talking to Slava if there is a better solution for the changes in the ES producer file.

@GNiendorf GNiendorf force-pushed the CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles_TrackLooperPR409 branch from feb65fe to bec054f Compare June 25, 2024 18:45
@GNiendorf GNiendorf force-pushed the CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles_TrackLooperPR409 branch from bec054f to 0c5c484 Compare June 26, 2024 12:17
@GNiendorf
Copy link
Member Author

/run all

Copy link

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     43.6    321.8    122.9     47.2     95.0    500.8    119.2    160.6    104.9      1.7    1517.8     973.4+/- 260.4     415.3   explicit_cache[s=4] (target branch)
   avg     47.9    328.4    117.9     47.4     95.1    506.8    118.0    159.9    103.8      2.2    1527.4     972.7+/- 257.5     418.1   explicit_cache[s=4] (this PR)

Copy link

There was a problem while building and running with CMSSW. The logs can be found here.

@GNiendorf
Copy link
Member Author

/run cmssw

Copy link

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

@GNiendorf
Copy link
Member Author

/run all

Copy link

The PR was built and ran successfully in standalone mode. Here are some of the comparison plots.

Efficiency vs pT comparison Efficiency vs eta comparison
Fake rate vs pT comparison Fake rate vs eta comparison
Duplicate rate vs pT comparison Duplicate rate vs eta comparison

The full set of validation and comparison plots can be found here.

Here is a timing comparison:

   Evt    Hits       MD       LS      T3       T5       pLS       pT5      pT3      TC       Reset    Event     Short             Rate
   avg     44.0    324.9    124.0     49.7     95.8    501.3    119.4    162.0    104.5      2.6    1528.3     983.0+/- 258.9     417.9   explicit_cache[s=4] (target branch)
   avg     47.2    322.4    119.9     49.7     97.6    510.3    121.3    161.9    105.5      1.9    1537.8     980.3+/- 260.2     422.3   explicit_cache[s=4] (this PR)

Copy link

The PR was built and ran successfully with CMSSW. Here are some plots.

OOTB All Tracks
Efficiency and fake rate vs pT, eta, and phi

The full set of validation and comparison plots can be found here.

@GNiendorf
Copy link
Member Author

@VourMa I think this PR is ready to be merged now.

Copy link
Collaborator

@VourMa VourMa left a comment

Choose a reason for hiding this comment

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

Thank you for the updates, @GNiendorf! Approving and merging.

@VourMa VourMa merged commit 013be2f into CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles_batch1_devel Jun 27, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants