Move Run 4 default to D127 - #51633
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51633/50465
|
|
A new Pull Request was created by @kpedro88 for master. It involves the following packages:
@AdrianoDee, @Alejandro1400, @DickyChant, @Dr15Jones, @EltonSh, @JanChyczynski, @antoniovagnerini, @arunhep, @bsunanda, @civanch, @cmsbuild, @davidlange6, @fabiocos, @francescobrivio, @ftenchini, @kfjack, @kpedro88, @makortel, @mandrenguyen, @mdhildreth, @miquork, @perrotta, @sroychow can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
-1 Failed Tests: RelVals Failed RelVals |
| defaultRun4Geometry = 'D127' | ||
| defaultDataSets['Run4D110']='CMSSW_15_1_0_pre5-150X_mcRun4_realistic_v1_STD_RegeneratedGS_Run4D110_noPU-v' | ||
| defaultDataSets['Run4D121']='CMSSW_20_0_0_pre1-150X_mcRun4_realistic_v1_STD_RegeneratedGS_D121_noPU-v' | ||
| defaultDataSets['Run4D127']='' |
There was a problem hiding this comment.
not sure what to do here, since we have not generated any RelVals with D127 yet. @cms-sw/pdmv-l2 @cms-sw/orp-l2
There was a problem hiding this comment.
I'm not sure pdmv is available at the moment. If the errors are just caused by missing inputs, then we can just ignore them.
|
I think the crash in 37834.0 is because of missing pileup input (though the stack trace points at The crash in the DD4hep workflow is more concerning. @bsunanda can you investigate? |
|
test parameters:
|
|
#51633 (comment) to avoid surprises from the HLT side |
|
The full trace for the segmentation violation in 37834.0 is: This looks like an instance of cms-hls4ml/hls4mlEmulatorExtras#7. I do not think it is directly related to this PR, but it is still concerning to encounter right before an MC campaign starts. |
|
The exception comes via that smells like an exception being thrown in the |
|
It might be related to early module destruction? |
No (even if the stack trace shows I'd suggest to run the job in a debugger and catch exceptions (or run it through |
|
@kpedro88 is there a backport already? Maybe we can get that going in parallel? |
|
See #51658 |
|
+db
|
|
+pdmv |
|
This pull request is fully signed and it will be integrated in one of the next master IBs (test failures were overridden). This pull request will now be reviewed by the release team before it's merged. @mandrenguyen, @sextonkennedy, @ftenchini (and backports should be raised in the release meeting by the corresponding L2) |
|
@mandrenguyen yes, this is not strictly needed. Also: when is the new release (for 20_1_X) expected? Just because with this in we will have to live with the input file error until we have it available to produce the new RelVals (+the time to produce them). |
We are overdue for a new 20_1_X pre-release (pre2). Should we go ahead and build the pre-release and then merge this after? Or should we include it in pre2? |
|
+1 |
can we remove the failing workflow from the PR matrix tests until the sample exists? |
I'd argue the failing workflow must be removed because the current failure prevents DQM and Reco comparisons from running and therefore prevents assessing whether PRs physics result impact is along expectations. |
This is indeed what I meant, as it seems no actions were taken so far, I went ahead at #51667 |
|
For the next time, could I suggest something along the following for the update process
|
This seems like an avoidable failure of our CI. Comparisons can be skipped for failing workflows and still generated for passing workflows.
Primarily, there needs to be a defined responsibility, assigned to a specific coordination / L2 area, to organize the timeline and process. Just having it written down is not sufficient. This responsibility used to be part of the L2 Upgrade Software Coordinator position and was not reassigned when that position was eliminated. More broadly, CMSSW testing requires both shorter and longer timelines than RelVals: we need samples quickly, and then we need them to last for a long time. In the long term, we should disconnect these two things because of the unavoidable impedance mismatch (or otherwise significantly reform the operational aspects). |
At the cost of shooting myself in the foot, I think this is "PdmV" / OV responsibility now. And the procedure proposed by Matti is for sure the right one. This was what I was mentioning above. Then, given the urgency driven by simulation and geometry, and the fact that the new release was coming shortly after, I think it was worth going ahead and merging this to solve the problem in ~24h, instead of doing a series of small PRs one after the other. After all we are in mid-August, and the PR rate is pretty low compared to usual; personally I supposed we could live with 24h of one failing wf. The ultimate solution is indeed already available in #51684. Then I agree with @kpedro88 about the fact that:
|
PR description:
Followup to #51601: move the default for central MC production.
(NB: the slew of test scripts that are not part of unit tests are not updated here.)
PR validation:
runTheMatrix.py workflows are updated accordingly. 37634.0 was tested locally and runs successfully.
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:
Will be backported to 20_0_X. (Requires #51620 to be merged and in an IB first.)