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.

Sign up for GitHub

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

MuonAlignment ASAN bug fix and DB output update #36145

Closed
wants to merge 1 commit into from
Closed

MuonAlignment ASAN bug fix and DB output update #36145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Alignment/MuonAlignment/interface/MuonAlignmentInputDB.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ class MuonAlignmentInputDB : public MuonAlignmentInputMethod {
const Alignments* dtAlignments,
const Alignments* cscAlignments,
const Alignments* gemAlignments,
const Alignments* globalPositionRcd,
const AlignmentErrorsExtended* dtAlignmentErrorsExtended,
const AlignmentErrorsExtended* cscAlignmentErrorsExtended,
const AlignmentErrorsExtended* gemAlignmentErrorsExtended);
const AlignmentErrorsExtended* gemAlignmentErrorsExtended,
const Alignments* globalPositionRcd);
~MuonAlignmentInputDB() override;

// ---------- const member functions ---------------------
Expand All @@ -67,10 +67,10 @@ class MuonAlignmentInputDB : public MuonAlignmentInputMethod {
const Alignments* dtAlignments_;
const Alignments* cscAlignments_;
const Alignments* gemAlignments_;
const Alignments* globalPositionRcd_;
const AlignmentErrorsExtended* dtAlignmentErrorsExtended_;
const AlignmentErrorsExtended* cscAlignmentErrorsExtended_;
const AlignmentErrorsExtended* gemAlignmentErrorsExtended_;
const Alignments* globalPositionRcd_;

const bool m_getAPEs;
};
Expand Down
20 changes: 19 additions & 1 deletion Alignment/MuonAlignment/plugins/MuonGeometryDBConverter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class MuonGeometryDBConverter : public edm::one::EDAnalyzer<> {
bool m_done;
std::string m_input, m_output;

std::string m_dtLabel, m_cscLabel, m_gemLabel;
std::string m_dtLabel, m_cscLabel, m_gemLabel, m_dtAPELabel, m_cscAPELabel, m_gemAPELabel;
double m_shiftErr, m_angleErr;
std::string m_fileName;
bool m_getAPEs;
Expand All @@ -76,6 +76,11 @@ class MuonGeometryDBConverter : public edm::one::EDAnalyzer<> {
edm::ESGetToken<Alignments, DTAlignmentRcd> dtAliToken_;
edm::ESGetToken<Alignments, CSCAlignmentRcd> cscAliToken_;
edm::ESGetToken<Alignments, GEMAlignmentRcd> gemAliToken_;

edm::ESGetToken<AlignmentErrorsExtended, DTAlignmentErrorExtendedRcd> dtAPEToken_;
edm::ESGetToken<AlignmentErrorsExtended, CSCAlignmentErrorExtendedRcd> cscAPEToken_;
edm::ESGetToken<AlignmentErrorsExtended, GEMAlignmentErrorExtendedRcd> gemAPEToken_;

const edm::ESGetToken<Alignments, GlobalPositionRcd> gprToken_;
};

Expand Down Expand Up @@ -116,6 +121,9 @@ MuonGeometryDBConverter::MuonGeometryDBConverter(const edm::ParameterSet &iConfi
m_dtLabel = iConfig.getParameter<std::string>("dtLabel");
m_cscLabel = iConfig.getParameter<std::string>("cscLabel");
m_gemLabel = iConfig.getParameter<std::string>("gemLabel");
m_dtAPELabel = iConfig.getParameter<std::string>("dtAPELabel");
m_cscAPELabel = iConfig.getParameter<std::string>("cscAPELabel");
m_gemAPELabel = iConfig.getParameter<std::string>("gemAPELabel");
m_shiftErr = iConfig.getParameter<double>("shiftErr");
m_angleErr = iConfig.getParameter<double>("angleErr");
m_getAPEs = iConfig.getParameter<bool>("getAPEs");
Expand All @@ -125,6 +133,10 @@ MuonGeometryDBConverter::MuonGeometryDBConverter(const edm::ParameterSet &iConfi
cscAliToken_ = esConsumes(edm::ESInputTag("", m_cscLabel));
gemAliToken_ = esConsumes(edm::ESInputTag("", m_gemLabel));

dtAPEToken_ = esConsumes(edm::ESInputTag("", m_dtAPELabel));
cscAPEToken_ = esConsumes(edm::ESInputTag("", m_cscAPELabel));
gemAPEToken_ = esConsumes(edm::ESInputTag("", m_gemAPELabel));

dtGeomToken_ = esConsumes(edm::ESInputTag("", idealGeometryLabelForInputXML));
cscGeomToken_ = esConsumes(edm::ESInputTag("", idealGeometryLabelForInputXML));
gemGeomToken_ = esConsumes(edm::ESInputTag("", idealGeometryLabelForInputXML));
Expand Down Expand Up @@ -173,6 +185,9 @@ void MuonGeometryDBConverter::analyze(const edm::Event &iEvent, const edm::Event
&iSetup.getData(dtAliToken_),
&iSetup.getData(cscAliToken_),
&iSetup.getData(gemAliToken_),
&iSetup.getData(dtAPEToken_),
&iSetup.getData(cscAPEToken_),
&iSetup.getData(gemAPEToken_),
&iSetup.getData(gprToken_));
MuonAlignment *muonAlignment = new MuonAlignment(iSetup, inputMethod);
if (m_getAPEs) {
Expand Down Expand Up @@ -216,6 +231,9 @@ void MuonGeometryDBConverter::fillDescriptions(edm::ConfigurationDescriptions &d
desc.add<std::string>("dtLabel", "");
desc.add<std::string>("cscLabel", "");
desc.add<std::string>("gemLabel", "");
desc.add<std::string>("dtAPELabel", "");
desc.add<std::string>("cscAPELabel", "");
desc.add<std::string>("gemAPELabel", "");
desc.add<double>("shiftErr", 1000.0);
desc.add<double>("angleErr", 6.28);
desc.add<bool>("getAPEs", true);
Expand Down
4 changes: 2 additions & 2 deletions Alignment/MuonAlignment/plugins/MuonGeometrySVGTemplate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include <fstream>

#include "FWCore/Framework/interface/Frameworkfwd.h"
#include "FWCore/Framework/interface/EDAnalyzer.h"
#include "FWCore/Framework/interface/one/EDAnalyzer.h"
#include "FWCore/Framework/interface/EventSetup.h"
#include "FWCore/Framework/interface/ESHandle.h"
#include "FWCore/Framework/interface/Event.h"
Expand All @@ -43,7 +43,7 @@
// class decleration
//

class MuonGeometrySVGTemplate : public edm::EDAnalyzer {
class MuonGeometrySVGTemplate : public edm::one::EDAnalyzer<> {
public:
explicit MuonGeometrySVGTemplate(const edm::ParameterSet &iConfig);
~MuonGeometrySVGTemplate() override;
Expand Down
38 changes: 19 additions & 19 deletions Alignment/MuonAlignment/plugins/MuonMisalignedProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ class MuonMisalignedProducer : public edm::one::EDAnalyzer<> {
edm::ESGetToken<CSCGeometry, MuonGeometryRecord> esTokenCSC_;
edm::ESGetToken<GEMGeometry, MuonGeometryRecord> esTokenGEM_;

Alignments dt_Alignments;
AlignmentErrorsExtended dt_AlignmentErrorsExtended;
Alignments csc_Alignments;
AlignmentErrorsExtended csc_AlignmentErrorsExtended;
Alignments gem_Alignments;
AlignmentErrorsExtended gem_AlignmentErrorsExtended;
Alignments* dt_Alignments;
AlignmentErrorsExtended* dt_AlignmentErrorsExtended;
Alignments* csc_Alignments;
AlignmentErrorsExtended* csc_AlignmentErrorsExtended;
Alignments* gem_Alignments;
AlignmentErrorsExtended* gem_AlignmentErrorsExtended;
};

//__________________________________________________________________________________________________
Expand All @@ -84,7 +84,7 @@ MuonMisalignedProducer::MuonMisalignedProducer(const edm::ParameterSet& p)
esTokenGEM_(esConsumes(edm::ESInputTag("", "idealForMuonMisalignedProducer"))) {}

//__________________________________________________________________________________________________
MuonMisalignedProducer::~MuonMisalignedProducer() = default;
MuonMisalignedProducer::~MuonMisalignedProducer() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the = default; is the recommended way of doing this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code-format tool changed this. do you want to keep = default;?

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, @makortel @smuzaffar I thought the default was the preferred option

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @hyunyong I wrote to you privately, you did many things in the PR from which some are correct and some are undoing the work of @mmusich about the DB output module changes

Oh, ok.
I copied the module from an old version.
I understood now what happened.
I will close this PR and make a new one.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is, and I'm a bit surprised that code-checks would change away from = default. We have plenty of those in .cc files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is, and I'm a bit surprised that code-checks would change away from = default. We have plenty of those in .cc files.

no, it was my mistake.


//__________________________________________________________________________________________________
void MuonMisalignedProducer::analyze(const edm::Event& event, const edm::EventSetup& eventSetup) {
Expand All @@ -102,12 +102,12 @@ void MuonMisalignedProducer::analyze(const edm::Event& event, const edm::EventSe
scenarioBuilder.applyScenario(theScenario);

// Get alignments and errors
dt_Alignments = *(theAlignableMuon->dtAlignments());
dt_AlignmentErrorsExtended = *(theAlignableMuon->dtAlignmentErrorsExtended());
csc_Alignments = *(theAlignableMuon->cscAlignments());
csc_AlignmentErrorsExtended = *(theAlignableMuon->cscAlignmentErrorsExtended());
gem_Alignments = *(theAlignableMuon->gemAlignments());
gem_AlignmentErrorsExtended = *(theAlignableMuon->gemAlignmentErrorsExtended());
dt_Alignments = theAlignableMuon->dtAlignments();
dt_AlignmentErrorsExtended = theAlignableMuon->dtAlignmentErrorsExtended();
csc_Alignments = theAlignableMuon->cscAlignments();
csc_AlignmentErrorsExtended = theAlignableMuon->cscAlignmentErrorsExtended();
gem_Alignments = theAlignableMuon->gemAlignments();
gem_AlignmentErrorsExtended = theAlignableMuon->gemAlignmentErrorsExtended();

// Misalign the EventSetup geometry
/* GeometryAligner aligner;
Expand All @@ -132,17 +132,17 @@ void MuonMisalignedProducer::saveToDB(void) {
throw cms::Exception("NotAvailable") << "PoolDBOutputService not available";

// Store DT alignments and errors
poolDbService->writeOneIOV<Alignments>(dt_Alignments, poolDbService->beginOfTime(), theDTAlignRecordName);
poolDbService->writeOneIOV<Alignments>(*dt_Alignments, poolDbService->beginOfTime(), theDTAlignRecordName);
poolDbService->writeOneIOV<AlignmentErrorsExtended>(
dt_AlignmentErrorsExtended, poolDbService->beginOfTime(), theDTErrorRecordName);
*dt_AlignmentErrorsExtended, poolDbService->beginOfTime(), theDTErrorRecordName);

// Store CSC alignments and errors
poolDbService->writeOneIOV<Alignments>(csc_Alignments, poolDbService->beginOfTime(), theCSCAlignRecordName);
poolDbService->writeOneIOV<Alignments>(*csc_Alignments, poolDbService->beginOfTime(), theCSCAlignRecordName);
poolDbService->writeOneIOV<AlignmentErrorsExtended>(
csc_AlignmentErrorsExtended, poolDbService->beginOfTime(), theCSCErrorRecordName);
poolDbService->writeOneIOV<Alignments>(gem_Alignments, poolDbService->beginOfTime(), theGEMAlignRecordName);
*csc_AlignmentErrorsExtended, poolDbService->beginOfTime(), theCSCErrorRecordName);
poolDbService->writeOneIOV<Alignments>(*gem_Alignments, poolDbService->beginOfTime(), theGEMAlignRecordName);
poolDbService->writeOneIOV<AlignmentErrorsExtended>(
gem_AlignmentErrorsExtended, poolDbService->beginOfTime(), theGEMErrorRecordName);
*gem_AlignmentErrorsExtended, poolDbService->beginOfTime(), theGEMErrorRecordName);
}
//____________________________________________________________________________________________
DEFINE_FWK_MODULE(MuonMisalignedProducer);
48 changes: 24 additions & 24 deletions Alignment/MuonAlignment/src/MuonAlignment.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,8 @@ void MuonAlignment::saveDTSurveyToDB(void) {
throw cms::Exception("NotAvailable") << "PoolDBOutputService not available";

// Get alignments and errors
Alignments dtAlignments{};
SurveyErrors dtSurveyErrors{};
Alignments* dtAlignments = new Alignments();
SurveyErrors* dtSurveyErrors = new SurveyErrors();

align::Alignables alignableList;
recursiveList(theAlignableMuon->DTBarrel(), alignableList);
Expand All @@ -268,13 +268,13 @@ void MuonAlignment::saveDTSurveyToDB(void) {
(*alignable)->id());
SurveyError error((*alignable)->alignableObjectId(), (*alignable)->id(), (*alignable)->survey()->errors());

dtAlignments.m_align.push_back(value);
dtSurveyErrors.m_surveyErrors.push_back(error);
dtAlignments->m_align.push_back(value);
dtSurveyErrors->m_surveyErrors.push_back(error);
}

// Store DT alignments and errors
poolDbService->writeOneIOV<Alignments>(dtAlignments, poolDbService->currentTime(), theDTSurveyRecordName);
poolDbService->writeOneIOV<SurveyErrors>(dtSurveyErrors, poolDbService->currentTime(), theDTSurveyErrorRecordName);
poolDbService->writeOneIOV<Alignments>(*dtAlignments, poolDbService->currentTime(), theDTSurveyRecordName);
poolDbService->writeOneIOV<SurveyErrors>(*dtSurveyErrors, poolDbService->currentTime(), theDTSurveyErrorRecordName);
}

void MuonAlignment::saveCSCSurveyToDB(void) {
Expand All @@ -284,8 +284,8 @@ void MuonAlignment::saveCSCSurveyToDB(void) {
throw cms::Exception("NotAvailable") << "PoolDBOutputService not available";

// Get alignments and errors
Alignments cscAlignments{};
SurveyErrors cscSurveyErrors{};
Alignments* cscAlignments = new Alignments();
Copy link
Contributor

Choose a reason for hiding this comment

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

here the original was the preferred solution

SurveyErrors* cscSurveyErrors = new SurveyErrors();

align::Alignables alignableList;
recursiveList(theAlignableMuon->CSCEndcaps(), alignableList);
Expand All @@ -301,13 +301,13 @@ void MuonAlignment::saveCSCSurveyToDB(void) {
(*alignable)->id());
SurveyError error((*alignable)->alignableObjectId(), (*alignable)->id(), (*alignable)->survey()->errors());

cscAlignments.m_align.push_back(value);
cscSurveyErrors.m_surveyErrors.push_back(error);
cscAlignments->m_align.push_back(value);
cscSurveyErrors->m_surveyErrors.push_back(error);
}

// Store CSC alignments and errors
poolDbService->writeOneIOV<Alignments>(cscAlignments, poolDbService->currentTime(), theCSCSurveyRecordName);
poolDbService->writeOneIOV<SurveyErrors>(cscSurveyErrors, poolDbService->currentTime(), theCSCSurveyErrorRecordName);
poolDbService->writeOneIOV<Alignments>(*cscAlignments, poolDbService->currentTime(), theCSCSurveyRecordName);
poolDbService->writeOneIOV<SurveyErrors>(*cscSurveyErrors, poolDbService->currentTime(), theCSCSurveyErrorRecordName);
}

void MuonAlignment::saveSurveyToDB(void) {
Expand All @@ -322,13 +322,13 @@ void MuonAlignment::saveDTtoDB(void) {
throw cms::Exception("NotAvailable") << "PoolDBOutputService not available";

// Get alignments and errors
Alignments dt_Alignments = *(theAlignableMuon->dtAlignments());
AlignmentErrorsExtended dt_AlignmentErrorsExtended = *(theAlignableMuon->dtAlignmentErrorsExtended());
const Alignments* dt_Alignments = theAlignableMuon->dtAlignments();
const AlignmentErrorsExtended* dt_AlignmentErrorsExtended = theAlignableMuon->dtAlignmentErrorsExtended();

// Store DT alignments and errors
poolDbService->writeOneIOV<Alignments>(dt_Alignments, poolDbService->currentTime(), theDTAlignRecordName);
poolDbService->writeOneIOV<Alignments>(*dt_Alignments, poolDbService->currentTime(), theDTAlignRecordName);
poolDbService->writeOneIOV<AlignmentErrorsExtended>(
dt_AlignmentErrorsExtended, poolDbService->currentTime(), theDTErrorRecordName);
*dt_AlignmentErrorsExtended, poolDbService->currentTime(), theDTErrorRecordName);
}

void MuonAlignment::saveCSCtoDB(void) {
Expand All @@ -338,13 +338,13 @@ void MuonAlignment::saveCSCtoDB(void) {
throw cms::Exception("NotAvailable") << "PoolDBOutputService not available";

// Get alignments and errors
Alignments csc_Alignments = *(theAlignableMuon->cscAlignments());
AlignmentErrorsExtended csc_AlignmentErrorsExtended = *(theAlignableMuon->cscAlignmentErrorsExtended());
const Alignments* csc_Alignments = theAlignableMuon->cscAlignments();
const AlignmentErrorsExtended* csc_AlignmentErrorsExtended = theAlignableMuon->cscAlignmentErrorsExtended();

// Store CSC alignments and errors
poolDbService->writeOneIOV<Alignments>(csc_Alignments, poolDbService->currentTime(), theCSCAlignRecordName);
poolDbService->writeOneIOV<Alignments>(*csc_Alignments, poolDbService->currentTime(), theCSCAlignRecordName);
poolDbService->writeOneIOV<AlignmentErrorsExtended>(
csc_AlignmentErrorsExtended, poolDbService->currentTime(), theCSCErrorRecordName);
*csc_AlignmentErrorsExtended, poolDbService->currentTime(), theCSCErrorRecordName);
}

void MuonAlignment::saveGEMtoDB(void) {
Expand All @@ -354,13 +354,13 @@ void MuonAlignment::saveGEMtoDB(void) {
throw cms::Exception("NotAvailable") << "PoolDBOutputService not available";

// Get alignments and errors
Alignments gem_Alignments = *(theAlignableMuon->gemAlignments());
AlignmentErrorsExtended gem_AlignmentErrorsExtended = *(theAlignableMuon->gemAlignmentErrorsExtended());
const Alignments* gem_Alignments = theAlignableMuon->gemAlignments();
const AlignmentErrorsExtended* gem_AlignmentErrorsExtended = theAlignableMuon->gemAlignmentErrorsExtended();

// Store CSC alignments and errors
poolDbService->writeOneIOV<Alignments>(gem_Alignments, poolDbService->currentTime(), theGEMAlignRecordName);
poolDbService->writeOneIOV<Alignments>(*gem_Alignments, poolDbService->currentTime(), theGEMAlignRecordName);
poolDbService->writeOneIOV<AlignmentErrorsExtended>(
gem_AlignmentErrorsExtended, poolDbService->currentTime(), theGEMErrorRecordName);
*gem_AlignmentErrorsExtended, poolDbService->currentTime(), theGEMErrorRecordName);
}

void MuonAlignment::saveToDB(void) {
Expand Down
6 changes: 3 additions & 3 deletions Alignment/MuonAlignment/src/MuonAlignmentInputDB.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,20 @@ MuonAlignmentInputDB::MuonAlignmentInputDB(const DTGeometry* dtGeometry,
const Alignments* dtAlignments,
const Alignments* cscAlignments,
const Alignments* gemAlignments,
const Alignments* globalPositionRcd,
const AlignmentErrorsExtended* dtAlignmentErrorsExtended,
const AlignmentErrorsExtended* cscAlignmentErrorsExtended,
const AlignmentErrorsExtended* gemAlignmentErrorsExtended)
const AlignmentErrorsExtended* gemAlignmentErrorsExtended,
const Alignments* globalPositionRcd)
: dtGeometry_(dtGeometry),
cscGeometry_(cscGeometry),
gemGeometry_(gemGeometry),
dtAlignments_(dtAlignments),
cscAlignments_(cscAlignments),
gemAlignments_(gemAlignments),
globalPositionRcd_(globalPositionRcd),
dtAlignmentErrorsExtended_(dtAlignmentErrorsExtended),
cscAlignmentErrorsExtended_(cscAlignmentErrorsExtended),
gemAlignmentErrorsExtended_(gemAlignmentErrorsExtended),
globalPositionRcd_(globalPositionRcd),
m_getAPEs(true) {}

// MuonAlignmentInputDB::MuonAlignmentInputDB(const MuonAlignmentInputDB& rhs)
Expand Down
4 changes: 2 additions & 2 deletions Alignment/MuonAlignmentAlgorithms/plugins/CSCOverlapsBeamSplashCut.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

// user include files
#include "FWCore/Framework/interface/Frameworkfwd.h"
#include "FWCore/Framework/interface/EDFilter.h"
#include "FWCore/Framework/interface/one/EDFilter.h"
#include "FWCore/Framework/interface/Event.h"
#include "FWCore/Framework/interface/EventSetup.h"
#include "FWCore/Framework/interface/ESHandle.h"
Expand All @@ -37,7 +37,7 @@
// class decleration
//

class CSCOverlapsBeamSplashCut : public edm::EDFilter {
class CSCOverlapsBeamSplashCut : public edm::one::EDFilter<> {
public:
explicit CSCOverlapsBeamSplashCut(const edm::ParameterSet&);
~CSCOverlapsBeamSplashCut() override;
Expand Down