-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[12_4_HLT_X] Improve memory usage in ParameterSet #45000
[12_4_HLT_X] Improve memory usage in ParameterSet #45000
Conversation
- store strings directly with trailing \0 instead of converting to hex values first. - replaced cppunit with catch2 test
Matti: FWCore/Integration/test/unit_test_outputs/testGetBy1.log had conflicts, because 12_4_X version does not include all the printouts the 13_0_X includes.
Change to PSet string encodings temporarily broke this. This will now test it works.
Caused by backporting cms-sw#42742 on top of cms-sw#43898
A new Pull Request was created by @makortel for CMSSW_12_4_HLT_X. It involves the following packages:
@tjavaid, @Dr15Jones, @syuvivida, @rvenditti, @smuzaffar, @antoniovagnerini, @cmsbuild, @mdhildreth, @makortel, @nothingface0, @civanch can you please review it and eventually sign? Thanks. cms-bot commands are listed here
|
cms-bot internal usage |
@cmsbuild, please test |
@cms-sw/pdmv-l2 Please note the |
@smuzaffar Are the tests stuck? |
On it, thanks! |
please test @makortel , yes tests were stuck as there was no CMSSW_12_4_X_2024-05-14-1100 IB ( for baseline generation) for CMSSW_12_4_HLT_X_2024-05-14-1100. lets re-run so that PR can use CMSSW_12_4_X_2024-05-19-0000 |
-1 Failed Tests: UnitTests Unit TestsI found 2 errors in the following unit tests: ---> test test-das-selected-lumis had ERRORS ---> test test_edmPickEvents had ERRORS Comparison SummarySummary:
|
Both unit test failures are
that looks transient |
+core |
backport of #42742 |
+1 |
@cms-sw/dqm-l2 Could you review and sign? Thanks! |
@makortel so this error is coming from the fact that the L1T menu loaded from the I have set up a possible workaround forcing the Also because, on the bright side, if I run the
the chain runs smoothly. So most probably it's not really needed. |
@cms-sw/dqm-l2 Could you please review and sign? Thanks! |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_4_HLT_X IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
@cms-sw/orp-l2 Here is my request on the 13_0_HLT_X and 12_4_HLT_X releases that I brought up in the ORP yesterday
@AdrianoDee I won't be able to update the tests in 14_1_X/14_0_X to use these 13_0_19_HLT and 12_4_21_HLT release until July, so if you need to make progress sooner, someone else has to update the tests. |
+1 |
merge |
And the 13_0_HLT_X PR is already merged (#44921). |
PR description:
This PR backports #42742 in order to allow a file written in 14_0_X to be read by a 12_4_X
cmsRun
in order to run the HLT step in 12_4_X in the upcoming 2022 MC campaign.The last commit was needed to get the backport to compile, because of #43898 was in 13_0_X already before the backport.
Note that files written with a release containing this PR will be unreadable by earlier 12_4_X releases. Therefore this PR is to be merged only in the 12_4_HLT_X branch.
Resolves cms-sw/framework-team#919
PR validation:
I modified the test_MC_22_setup test, added in #44578, to use my local 12_4_20 + this PR developer area, and the test got beyond the ParameterSet-related error. The job still failed with another error
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
Backport of #42742