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

[GEN Check Script] Adding a number of events check for pLHE #3816

Open
DickyChant opened this issue Dec 9, 2024 · 9 comments
Open

[GEN Check Script] Adding a number of events check for pLHE #3816

DickyChant opened this issue Dec 9, 2024 · 9 comments

Comments

@DickyChant
Copy link
Contributor

Recently we noticed that in production there could be requests mistakenly requesting more events than presented private LHE events. See [1] for an example.

Without a check we (GEN and PdmV) could not effectively identify this issue... We should brainstorm on how we could add this check.

  1. The most straightforward way would be reading the whole files, though it would be very heavy workload
  2. @vlimant proposed to skip events until the final one 1k event with a cmsDriver command.

More ideas are welcomed for sure.

@bbilin
Copy link
Collaborator

bbilin commented Dec 9, 2024

Hi,

I chose a random folder with many files: WJets sample at 8 TeV mcdbid: 5497.

The simple
[bbilin@lxplus9110 5497]$ grep "</event>" *lhe |wc -l

Returned the nevents =107350000 in a timely manner. This can hence indeed be included in the script to check nreq<= n available.

Cheers,

B.

@DickyChant
Copy link
Contributor Author

I think one of the issue with this solution is however that we could have ".xz" compressed LHE files :(

@bbilin
Copy link
Collaborator

bbilin commented Dec 9, 2024

ah, at least vim was working on them, we can find a way out :)

@bbilin
Copy link
Collaborator

bbilin commented Dec 9, 2024

google helped. xzgrep works.

@DickyChant
Copy link
Contributor Author

Look forward to a PR to close this then ;)

@agrohsje
Copy link
Collaborator

agrohsje commented Dec 9, 2024

Just a reminder that vim created spurious copies in the past and screwed the system. Keep also in mind when running locally.

@bbilin
Copy link
Collaborator

bbilin commented Dec 9, 2024

yes, it causes problems in the event generation. We had to remove those swp files. (I am not using vim by default :) )

@DickyChant
Copy link
Contributor Author

I forget to have [1]: https://its.cern.ch/jira/browse/HIGHPRIOREQ-825 ...

@Kiarendil
Copy link

Hi all,
the current check by @efeyazgan in #3817 xzgrep "</event>" /eos/cms/store/lhe/'+str(mcdbid)+'/*.lhe | wc -l does not allow to count the events within ".xz" compressed LHE files — obviously, it checks only ".lhe"
However, xzgrep "</event>" /eos/cms/store/lhe/'+str(mcdbid)+'/*.lhe* | wc -l works for them.
Thus, maybe such fix with an extra wild card is needed.

I have faced this issue with the validation failed for BPH-RunIII2024Summer24pLHEGS-00001.

While I fully support the need of such a check for the production, I'm not happy with the current implementation within request_fragment_check.py script. It works only before the validation step at McM side or if MC contact uses script manually (as they should always do, of course...).
The problem is MC contacts can change events number for "validation" and "defined" McM statuses after validation, and the current implementation could not help with it.
Personally as MC contact I almost always adjust event number after the validation, thus it is easy to miss this check.

Also, the current check could take up to 10-20 minutes for the large folders (it took so for me for e.g. 20022 and 5497 ids). I'm not sure it is OK for the fast validation check.

Thus I would suggest maybe to remove this check from the request_fragment_check.py script to not waste (sometimes big) time on this (however leave a warning for all pLHE PrepIDs to check event number with xzgrep "</event>" /eos/cms/store/lhe/'+str(mcdbid)+'/*.lhe* | wc -l command) and rather implement this check on the McM side during approval / injection.
E.g. McM have checks during the injection to avoid identical dataset names. It should also check this for pLHE samples, this would be much better rather than current check during the validation.

Best,
Kirill

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

No branches or pull requests

4 participants