added NanoAOD to PromptReco

[drkovalskyi]

PR description:

Added NanoAOD option to the prompt reco configuration building to be used at Tier0. This PR is critical for NanoAOD integration at Tier0 and targets first collisions. Other developments outside CMSSW (in WMCore) are on hold till we have a working pre-release or IB to use in Tier0 replays.

PR validation:

• Executed scram unit tests
• Tested that one can create a working configuration that produces valid output

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37728/29578

• This PR adds an extra 16KB to repository

• There are other open Pull requests which might conflict with changes you have proposed:

  • File Configuration/DataProcessing/python/Reco.py modified in PR(s): [12_4_X] simple RECO squence for SPLASH runs #37624

[cmsbuild]

A new Pull Request was created by @drkovalskyi for master.

It involves the following packages:

• Configuration/DataProcessing (operations)

@cmsbuild, @perrotta, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol, @fabiocos this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[perrotta]

please test

[perrotta]

Do I understand correctly that the unit tests should now pick up the new config, which will therefore get tested already in the IB tests then?

Do I understand correctly that this PR adds the nanoAODStep to the prompt reco by default?

You need eventually a backport to 12_3, I guess. As you know a 12_3_2 release was built just a few hours ago, mostly to satisfy the request from the T0 crew. What are the plans to deploy these further developments in the Tier0 release?

[drkovalskyi]

The tests should run as part of TestConfigDP. They are just making configurations, they are not trying to run them though, but they would catch incorrect configuration parameters.

Regarding nanoAODStep it will be optional as all other steps are in principle. It's just a configuration building tool.

Regarding back-porting to 12_3, it can be done, but I don't think it will be necessary if 12_4 will be our production release for stable beams. We won't need it earlier and we are happy to test with IB/pre-releases for now.

[davidlange6]

are there relvals/ (eg, runTheMatrix) workflows configured like this? cms should be testing the planned tier0 configuration..

[mariadalfonso]

are there relvals/ (eg, runTheMatrix) workflows configured like this? cms should be testing the planned tier0 configuration..

@[davidlange6]
we have one that runs mini and nano with Run2 inputs (not in the default run3)
https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/python/relval_steps.py#L3297-L3305

we should eventually add more for run3

[drkovalskyi]

The only thing that is not being tested in RelVals or the matrix is the simultaneous running of reco, pat and nano. This would be good to test. I can add a test like that to the matrix.

[drkovalskyi]

Thanks @mariadalfonso. It would be good to have a test that has all 3 components, i.e. reco, pat and nano.

[davidlange6]

Yes, exactly the point. But its not just a run-without-crashing test that should be done here

…

On Apr 29, 2022, at 1:16 PM, drkovalskyi ***@***.***> wrote: The only thing that is not being tested in RelVals or the matrix is the simultaneous running or reco, pat and nano. This would be good to test. I can add a test like that to the matrix. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[drkovalskyi]

Yes, exactly the point. But its not just a run-without-crashing test that should be done here

There are many things that would be great to do. I would prefer to focus on the task at hands at the moment though. We can expand Tier0 testing in future.

[davidlange6]

Surely we must test what we run... not saying "you" should do it of course..

…

On Apr 29, 2022, at 1:30 PM, drkovalskyi ***@***.***> wrote: Yes, exactly the point. But its not just a run-without-crashing test that should be done here There are many things that would be great to do. I would prefer to focus on the task at hands at the moment though. We can expand Tier0 testing in future. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9f1e08/24328/summary.html
COMMIT: 5a4da5e
CMSSW: CMSSW_12_4_X_2022-04-28-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37728/24328/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 3 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3695434
• DQMHistoTests: Total failures: 8
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3695404
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
• Checked 205 log files, 45 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[perrotta]

@drkovalskyi @mariadalfonso should we expect such a test for Run3 to be added already to this PR, so that it can be tested (as working) at once with the new option?

[drkovalskyi]

@perrotta Having all 3 steps in one test is just a nice to have thing, because RECO and PAT are used/tested routinely at Tier0. PAT and NANO are tested by X-POG and it's the most non-trivial part. Let's integrate the changes so that we can continue integration.

[davidlange6]

This assumes that combining a+b yields the same results as running them separately... [wasn't immediately the case back when mini was added to reco for example]

…

On May 2, 2022, at 10:26 AM, drkovalskyi ***@***.***> wrote: @perrotta Having all 3 steps in one test is just a nice to have thing, because RECO and PAT are used/tested routinely at Tier0. PAT and NANO are tested by X-POG and it's the most non-trivial part. Let's integrate the changes so that we can continue integration. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[perrotta]

are there relvals/ (eg, runTheMatrix) workflows configured like this? cms should be testing the planned tier0 configuration..

@[davidlange6] we have one that runs mini and nano with Run2 inputs (not in the default run3) https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/python/relval_steps.py#L3297-L3305

we should eventually add more for run3

@mariadalfonso is that Run3 version of the workflow something that you can setup quickly? Do you think that it can be already included in this PR, to allow at least some initial "testability" for it?

[drkovalskyi]

@perrotta Could we just turn the request for additional tests into a github issue and proceed with the integration of this PR? I don't see anything urgent about having extra test implemented now and Maria may have many other urgent things to work on. At the same time this PR holds us from integrating NanoAOD into prompt reco at Tier0.

[mariadalfonso]

@perrotta @drkovalskyi
I think it's easier for me to add more WF Run3 mini/nano on a separate followup PR.
Also need to figure out the Run3 data we have for runTheMatrix/ Relvals .
MINI/NANO/both DQM Run2 is already there and tested at any PRs.

[perrotta]

+1

• The new option is added in view of further developments: some testing capability is expected to get provided in parallel
• Standard workflows are not affected

[cmsbuild]

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 be automatically merged.

[davidlange6]

I mean specifically that (some) things that get validated should match with the procedure done in production . So from raw to nano. Indeed, run3 is the most relevant configuration for this validation.

…

On Apr 29, 2022, at 1:13 PM, mariadalfonso ***@***.***> wrote: are there relvals/ (eg, runTheMatrix) workflows configured like this? cms should be testing the planned tier0 configuration.. @[davidlange6] we have one that runs mini and nano with Run2 inputs (not in the default run3) https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/python/relval_steps.py#L3297-L3305 we should eventually add more for run3 — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.