Skip to content

BPHNanoAODs #47469

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

Closed
wants to merge 6 commits into from
Closed

BPHNanoAODs #47469

wants to merge 6 commits into from

Conversation

gmelachr
Copy link
Contributor

Following the presentation in the xPOG meeting (https://indico.cern.ch/event/1509026/#2-bph-custom-nanoaod), this is the PR for the BPH-flavor nanoAODs.

FYI: @drkovalskyi @gkaratha @hqucms @ftorrresd

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 27, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47469/43900

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@vlimant
Copy link
Contributor

vlimant commented Mar 3, 2025

assign xpog

@vlimant
Copy link
Contributor

vlimant commented Mar 3, 2025

please include how this would be ran within a cmsDriver command ; and this will likely require the --step NANO:@BPH to be implemented.

@ftorrresd
Copy link
Contributor

enable nano

@ftorrresd
Copy link
Contributor

please test

@ftorrresd
Copy link
Contributor

Hi @gmelachr -- as a follow-up on Jean-Roch's comment, usually NanoAOD flavors implement their customizations and make them related to a new sequence at autoNANO.py

https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/autoNANO.py

@makortel
Copy link
Contributor

makortel commented Mar 7, 2025

The binary files should be placed in a (new?) repository in https://github.com/cms-data/

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a wrong package to define dictionaries for persistable data products (see https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideCreatingNewProducts#Restrictions_on_the_package_defi).

Comment on lines +3 to +7
<class name="reco::TransientTrack"/>
<class name="std::vector<reco::TransientTrack>" ClassVersion="6">
<version ClassVersion="6" checksum="1695337441"/>
</class>
<class name="edm::Wrapper<std::vector<reco::TransientTrack> >"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Dictionaries for reco::TransientTrack must be defined in the package that defines the class, i.e. TrackingTools/TransientTrack. Given that that package is a non-DataFormat package, the dictionary for Wrapper must be declared as transient along

 <class name="edm::Wrapper<std::vector<reco::TransientTrack> >" persistent="false"/>

The dictionary for std::vector<reco::TransientTrack> should not specify class version (because it is an instantiation of a class template).

Comment on lines +8 to +10
<class name="KinVtxFitter"/>
<class name="std::vector<KinVtxFitter>"/>
<class name="edm::Wrapper<std::vector<KinVtxFitter> >"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

If dictionary for KinVtxFitter is needed, the Wrapper must be declared as transient

 <class name="edm::Wrapper<std::vector<KinVtxFitter> >" persistent="false"/>

However, it seems not to be put into the edm::Event in the present form of this PR (I only saw commented-out code). If it is not necessary to put it into edm::Event, I'd suggest to remove that commented-out code and these dictionary declarations.

Comment on lines +12 to +14
<class name="edm::RefProd<pat::CompositeCandidateCollection>" />
<class name="edm::Association<pat::CompositeCandidateCollection>" />
<class name="edm::Wrapper<edm::Association<pat::CompositeCandidateCollection> >" />
Copy link
Contributor

Choose a reason for hiding this comment

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

These dictionaries must go to the package that defines pat::CompositeCandidateCollection, i.e. DataFormats/PatCandidates.

Comment on lines +12 to +18
namespace {
struct dictionary {
std::vector<reco::TransientTrack> ttv;
edm::Wrapper<std::vector<reco::TransientTrack> > wttv;
edm::Wrapper<std::vector<KinVtxFitter> > wkv;
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This dummy struct is not needed (since long time).

Suggested change
namespace {
struct dictionary {
std::vector<reco::TransientTrack> ttv;
edm::Wrapper<std::vector<reco::TransientTrack> > wttv;
edm::Wrapper<std::vector<KinVtxFitter> > wkv;
};
}

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47469/44011

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@gmelachr
Copy link
Contributor Author

Hi @gmelachr -- as a follow-up on Jean-Roch's comment, usually NanoAOD flavors implement their customizations and make them related to a new sequence at autoNANO.py

https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/NanoAOD/python/autoNANO.py

Hello, the BPH-flavor has been added in the autoNANO.py

@ftorrresd @vlimant The cmsDriver command is:

cmsDriver.py --conditions 140X_dataRun3_Prompt_v4 --datatier NANOAOD --era Run3,run3_nanoAOD_pre142X --eventcontent NANOAOD --filein root://cms-xrd-global.cern.ch//store/data/Run2024C/ParkingDoubleMuonLowMass0/MINIAOD/PromptReco-v1/000/379/415/00000/b40397b5-61c6-4887-8f4e-025e8ca925ee.root --fileout file:/tmp/gmelachr/BPH_test_data.root --nThreads 4 -n -1 --no_exec --python_filename BPH_test.py --scenario pp --step NANO:@bph

@makortel 's comments are work in progress

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47469/44039

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@gmelachr gmelachr closed this by deleting the head repository Mar 11, 2025
This was referenced Mar 15, 2025
@gmelachr
Copy link
Contributor Author

The review of the PR continues at #47603

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.

5 participants