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

Reconfigure interface for saving PF cands #31

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

DryRun
Copy link

@DryRun DryRun commented Mar 3, 2021

(I'm not sure this is the best place to talk about AK15 in general, maybe we should discuss this first)

Before trying to add AK15 PF candidates, this PR changes the arguments for addPFcands() and add_BTV() to specify which collections to save inclusively (addAK4, addAK8) instead of exclusively (onlyAK4, onlyAK8). This makes it easier to add another collection (addAK15), instead of enumerating the 2^(N jet collections) combinations.

Validation
Recreated all the CMSSW config files with make_configs_UL.sh, dumped the full configs with edmConfigDump, and confirmed that the configs are identical with diff, except for an intended change to the docstring ("from AK4 and AK8 jets" --> "from various jet collections").

@DryRun
Copy link
Author

DryRun commented Mar 3, 2021

I forgot to say, I also added some commented-out lines to add AK15 PF candidates as placeholders, but the AK15 jets don't actually exist anywhere yet.

@alefisico
Copy link
Member

I forgot to say, I also added some commented-out lines to add AK15 PF candidates as placeholders, but the AK15 jets don't actually exist anywhere yet.

Just to make sure I understand, the PR adds everything for ak15 jets to be included but does not include them yet? Is this correct?

@DryRun
Copy link
Author

DryRun commented Mar 8, 2021

Hi @alefisico - the commented AK15 lines are just copies of the AK8 lines, so I think they're everything needed to add AK15 PF candidates, once the AK15 jets themselves are added. I'm still working on adding the AK15 jets themselves, so the AK15 PF candidate code is not validated at all. Maybe I should remove them for now, and put them back once they're tested?

@alefisico
Copy link
Member

Hi @alefisico - the commented AK15 lines are just copies of the AK8 lines, so I think they're everything needed to add AK15 PF candidates, once the AK15 jets themselves are added. I'm still working on adding the AK15 jets themselves, so the AK15 PF candidate code is not validated at all. Maybe I should remove them for now, and put them back once they're tested?

Ah, yeah maybe, to avoid confusion. The other idea would be to leave this PR open until the AK15 jets are included and then we merge it. No strong opinion.

1 similar comment
@alefisico
Copy link
Member

Hi @alefisico - the commented AK15 lines are just copies of the AK8 lines, so I think they're everything needed to add AK15 PF candidates, once the AK15 jets themselves are added. I'm still working on adding the AK15 jets themselves, so the AK15 PF candidate code is not validated at all. Maybe I should remove them for now, and put them back once they're tested?

Ah, yeah maybe, to avoid confusion. The other idea would be to leave this PR open until the AK15 jets are included and then we merge it. No strong opinion.

@DryRun
Copy link
Author

DryRun commented Mar 12, 2021

OK, I have removed the AK15 template code. Can go back to 22b8fc0 if a reference is needed.

@alefisico
Copy link
Member

Hey @DryRun is this PR ready? I try to run it and I am getting errors like this:

----- Begin Fatal Exception 09-Jul-2021 11:35:10 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 1 lumi: 17359 event: 1735806 stream: 2
   [1] Running path 'NANOAODSIMoutput_step'
   [2] Prefetching for module NanoAODOutputModule/'NANOAODSIMoutput'
   [3] Prefetching for module GenJetConstituentTableProducer/'genAK15ConstituentsTable'
   [4] Calling method for module GenJetPackedConstituentPtrSelector/'genJetsAK15Constituents'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for a container with elements of type: reco::GenJet
Looking for module label: AK15GenJetsNoNu
Looking for productInstanceName: 

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------

when running process = PFnano_customizeMC_allPF(process)

@DryRun
Copy link
Author

DryRun commented Jul 9, 2021

Hi @alefisico (and @andrzejnovak) - sorry, not yet. Two problems:

  • I'm not sure if the code to cluster AK15 jets belongs in this repository. I did this following github.com/hqucms/NanoTuples, which uses jetToolbox and also adds a bunch of ParticleNet stuff. A "clone-in" procedure might be more appropriate, with the disadvantage is that then the AK15 code in addPFcands() will reference an AK15 jet collection created in some independent user code (should be disabled by default, anyways).

  • I've just realized that the logic in addPFcands() is a bit different from what I thought. If I understand correctly now, the AK4 and AK8 jet-PFcand association tables are always made, while the "onlyAK4" and "onlyAK8" arguments restrict the contents of the PFcands collection. I changed this so the tables and PFcand collection both follow the input args, i.e. saveAll=False, saveAK4=False, saveAK8=True will result in no AK4-PFcand association table. This is an unintended change; but maybe a reasonable one anyways?

@alefisico
Copy link
Member

Hi @alefisico (and @andrzejnovak) - sorry, not yet. Two problems:

* I'm not sure if the code to cluster AK15 jets belongs in this repository. I did this following github.com/hqucms/NanoTuples, which uses jetToolbox and also adds a bunch of ParticleNet stuff. A "clone-in" procedure might be more appropriate, with the disadvantage is that then the AK15 code in `addPFcands()` will reference an AK15 jet collection created in some independent user code (should be disabled by default, anyways).

* I've just realized that the logic in `addPFcands()` is a bit different from what I thought. If I understand correctly now, the AK4 and AK8 jet-PFcand association tables are always made, while the "onlyAK4" and "onlyAK8" arguments restrict the contents of the PFcands collection. I changed this so the tables and PFcand collection both follow the input args, i.e. `saveAll=False, saveAK4=False, saveAK8=True` will result in no AK4-PFcand association table. This is an unintended change; but maybe a reasonable one anyways?

Ah no problem, I was just wondering if you want me to merge it or not. Just let me know.
About 1) yeah I think that can be an external package that we set it up and we use it if needed.
About 2) I think so

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.

2 participants