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

Allow for ALL steps to be ran in one submission. #89

Merged
merged 4 commits into from
Nov 25, 2019

Conversation

selvaggi
Copy link
Contributor

@selvaggi selvaggi commented Nov 8, 2019

This enables the possibility of running the 3 steps together in one single submission by specifying the --datTier ALL option.

Copy link
Contributor

@clelange clelange left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me, I just have two questions/comments in-line, and will have to test this myself (tomorrow/Wednesday).

SubmitHGCalPGun.py Show resolved Hide resolved
SubmitHGCalPGun.py Show resolved Hide resolved
@clelange
Copy link
Contributor

Ah, OK, I didn't realise that only the NTUP part is written out. What about adding options to keep (i.e. copy) the intermediate steps as well? We could also address this in a separate PR later.

My current use case is actually to only run up to the RECO step, and I'd like to keep both GSD and RECO (the only reason for GSD being that we do not have a re-RECO step at the moment), so the ALL step doesn't address my use case, and I was wondering whether one could introduce a "allInOne" option instead of the ALL data tier, and allow for the data tier a comma-separated list that would determine the output to be stored. For example, --datTier NTUP --allInOne would run up to NTUP and only store NTUP, while --datTier GSD,RECO --allInOne would run up to RECO and also keep GSD. It's more work to implement this though, and we can first merge this PR.

Let me know what you prefer.

@selvaggi
Copy link
Contributor Author

selvaggi commented Nov 11, 2019

Yes, it is a good idea. But I would prefer we merge this first (after you have tested it), and then upgrade to what you suggest later. I have some others small items that I would like to add first.
Maybe you can add your suggestion to the PR, or create an issue so that we keep that in mind.

@franzoni
Copy link

hello @selvaggi @clelange
when this PR Was done, I could update and complete #87
perhaps you have an eta ?

@franzoni franzoni closed this Nov 13, 2019
@franzoni
Copy link

closed accidentally! Sorry.

@selvaggi
Copy link
Contributor Author

Trying to re-open.

@franzoni franzoni reopened this Nov 13, 2019
@franzoni
Copy link

thanks @selvaggi

@clelange
Copy link
Contributor

@franzoni I'm done in terms of code and documentation review, but haven't taken the time to actually take this branch and run a few test jobs.
I think an easy test would be to produce 2-3 events using the ALL option, and producing the same events running each step individually. In the end, this should give the same ntuple, while the step-by-step approach should also have intermediate output files. Would you like to try this and report back?

@clelange clelange merged commit a8e0b09 into CMS-HGCAL:master Nov 25, 2019
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.

None yet

3 participants