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 combined premixing stage1+stage2 workflows for 2016-18 #22956

Merged
merged 2 commits into from Apr 23, 2018

Conversation

makortel
Copy link
Contributor

This PR suggests an implementation for combined premixing stage1+stage2 workflows for 2016, 2017, and 2018, following discussions after my O&C week presentation on phase2 premixing
https://indico.cern.ch/event/711343/#54-phase-2-premixing-status

The combined workflow allows testing changes affecting stage1 such that the changes propagate to the DQM histograms of stage2.

The workflows are added for ttbar as a representative sample, and also for ZMM for 2018 to possibly help with the muon premixing debugging.

A speciality of these workflows is that first two steps (signal GEN-SIM and premix stage1) do not take any input, and the third step (premix stage2) takes the the input of the first two steps via --filein and --pileup_input (respectively). I did not find a clear way to achieve that (without spending lots of time understanding the details of the workflow generation), so I added a special treatment for premixing stage1 to WorkFlowRunner.

Tested in CMSSW_10_2_X_2018-04-12-2300, no changes expected in existing workflows.

@kpedro88 @mdhildreth

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@makortel
Copy link
Contributor Author

@cmsbuild, please test workflow 250202.1,250202.171,250202.181,250206.181

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 13, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/27481/console Started: 2018/04/13 17:16

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

Configuration/PyReleaseValidation

@GurpreetSinghChahal, @cmsbuild, @prebello, @kpedro88, @fabozzi can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @felicepantaleo, @ebrondol this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22956/27481/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-22956/250202.171_TTbar_13UP17+TTbar_13UP17+PREMIXUP17_PU25+DIGIPRMXLOCALUP17_PU25+RECOPRMXUP17_PU25+HARVESTUP17_PU25
  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-22956/250202.181_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25
  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-22956/250202.1_TTbar_13+TTbar_13+PREMIXUP15_PU25+DIGIPRMXLOCALUP15_PU25+RECOPRMXUP15_PU25+HARVESTUP15_PU25
  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-22956/250206.181_ZMM_13UP18+ZMM_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25_L1TMuDQM+HARVESTUP18_PU25_L1TMuDQM

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2409196
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2409028
  • DQMHistoTests: Total skipped: 167
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 21 files compared)
  • Checked 107 log files, 9 edm output root files, 27 DQM output files

@kpedro88
Copy link
Contributor

+1
it would be nice to have some generic way to tell WorkflowRunner when a step does not need an input file, but I don't immediately see a solution for that.

@makortel
Copy link
Contributor Author

it would be nice to have some generic way to tell WorkflowRunner when a step does not need an input file, but I don't immediately see a solution for that.

I agree, and wrote a bit more generic issue #22998.

@prebello
Copy link
Contributor

Hi @makortel , please I would like few clarifications.

  1. did you add these new relvals to be applied for PhaseII pmx validation only?
  2. On the other hand, you have mentioned that it can fix the muon pmx issues. So, does it mean that, if it works, Muon pmx relval should change?

Thank you

@makortel
Copy link
Contributor Author

  1. did you add these new relvals to be applied for PhaseII pmx validation only?

No, the workflows are added for 2016, 2017, and 2018.

The current separate workflows test the premixing procedure as in production (pileup library (stage1) and overlaying pileup with signal (stage2) separately). While this is a necessary test, it is not optimal for testing changes affecting stage1 (either accidentally or on purpose), as those would be seen only after next pre-release with the new pileup library. Combined workflow exercising both stage1 and stage2 tests the whole chain in one go, and is in my opinion the best way to resolve #22586. (obviously there is no need to run these workflows beyond PR tests and IBs, so RelVal procedure can continue as they are)

For phase2 there is no stage2 implementation yet (it's coming but I need bunch of pre-requisite PRs to be merged first). A combined stage1+stage2 workflow for phase2 will be included as well (actually being the only way to test the whole chain until the first pileup library, and even then stage1 may be a bit "unstable" for some time).

  1. On the other hand, you have mentioned that it can fix the muon pmx issues. So, does it mean that, if it works, Muon pmx relval should change?

No. At the time of submitting this PR I thought the fixes needed for CSC, DT, and RPC digi validation would have required breaking stage2-only workflows or disabling these modules in these workflows (see discussion #22957), but that turned out to be a misunderstanding from my part (see the actual fix in #22977). In case the stage2-only workflows would have been affected a combined stage1+stage2 workflow for ZMM would have been useful to produce histograms for these validation modules from events with muons. Since that didn't happen there is little motivation for the ZMM workflow (since ttbar has also muons, even if less). I can drop it if people prefer that (one workflow less to be run in IBs).

@fabozzi
Copy link
Contributor

fabozzi commented Apr 20, 2018

+1

@cmsbuild
Copy link
Contributor

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 now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

# though) also for "-i all" because in that case the --filein for DAS
# input is after this one in the list of command line arguments to
# cmsDriver, and gets then used in practice.
digiPremixLocalPileup = {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this going to work in a relval setup?

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume you mean that this works for the single workflow but not necessarily when injecting the production of the overall sample. I think this is a question for PdmV, how is this managed there?

Copy link
Contributor

Choose a reason for hiding this comment

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

@davidlange6
I do not think these kinds of premix workflows can run as a relval, because in a taskchain all the output datasets from the workflow steps are being produced at the same time (and not one after the other). So the mixing step will search for an input PU dataset that is still in production. In addition to this, I am not sure if it is possible to specify in WMAgent an input PU taken from a previous step of the workflow.

But I think these workflows are only needed for testing a PR or to run in IB, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I think these workflows are only needed for testing a PR or to run in IB, right?

That (PR+IB) is indeed my intention. I'd think the current separate stages approach works indeed better for RelVal (and production).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, let's take this anyway in mind

@fabiocos
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants