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

Replace reference to ParameterSet with explicit parameter values in HBHEDarkening #42528

Merged
merged 2 commits into from
Aug 15, 2023
+37 −28
Merged

Replace reference to ParameterSet with explicit parameter values in HBHEDarkening #42528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ef6285
Avoid copies of std::map and std::vector in HBHEDarkening construction
makortel Aug 9, 2023
a0ad094
Replace reference to ParameterSet with explicit parameter values
makortel Aug 9, 2023
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
45 changes: 23 additions & 22 deletions CalibCalorimetry/HcalPlugins/src/HBHEDarkeningEP.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,28 @@
#include <vector>
#include <map>

HBHEDarkeningEP::HBHEDarkeningEP(const edm::ParameterSet& pset) : pset_(pset) {
HBHEDarkeningEP::HBHEDarkeningEP(const edm::ParameterSet& pset)
: drdA_(pset.getParameter<double>("drdA")),
drdB_(pset.getParameter<double>("drdB")),
ieta_shift_(pset.getParameter<int>("ieta_shift")) {
setWhatProduced(this);
findingRecord<HBHEDarkeningRecord>();

const std::vector<edm::ParameterSet>& p_dosemaps = pset.getParameter<std::vector<edm::ParameterSet>>("dosemaps");
dosemaps_.reserve(p_dosemaps.size());
for (const auto& p_dosemap : p_dosemaps) {
dosemaps_.push_back({p_dosemap.getParameter<edm::FileInPath>("file"), p_dosemap.getParameter<int>("energy")});
}

//initialize years
const std::vector<edm::ParameterSet>& p_years = pset.getParameter<std::vector<edm::ParameterSet>>("years");
years_.reserve(p_years.size());
for (const auto& p_year : p_years) {
years_.emplace_back(p_year.getParameter<std::string>("year"),
p_year.getParameter<double>("intlumi"),
p_year.getParameter<double>("lumirate"),
p_year.getParameter<int>("energy"));
}
}

HBHEDarkeningEP::~HBHEDarkeningEP() {}
Expand Down Expand Up @@ -45,30 +64,12 @@ void HBHEDarkeningEP::fillDescriptions(edm::ConfigurationDescriptions& descripti
// ------------ method called to produce the data ------------
HBHEDarkeningEP::ReturnType HBHEDarkeningEP::produce(const HBHEDarkeningRecord& iRecord) {
//initialize dose maps
std::vector<edm::ParameterSet> p_dosemaps = pset_.getParameter<std::vector<edm::ParameterSet>>("dosemaps");
std::map<int, std::vector<std::vector<float>>> dosemaps;
for (const auto& p_dosemap : p_dosemaps) {
edm::FileInPath fp = p_dosemap.getParameter<edm::FileInPath>("file");
int file_energy = p_dosemap.getParameter<int>("energy");
dosemaps.emplace(file_energy, HBHEDarkening::readDoseMap(fp.fullPath()));
}

//initialize years
std::vector<edm::ParameterSet> p_years = pset_.getParameter<std::vector<edm::ParameterSet>>("years");
std::vector<HBHEDarkening::LumiYear> years;
years.reserve(p_years.size());
for (const auto& p_year : p_years) {
years.emplace_back(p_year.getParameter<std::string>("year"),
p_year.getParameter<double>("intlumi"),
p_year.getParameter<double>("lumirate"),
p_year.getParameter<int>("energy"));
for (const auto& p_dosemap : dosemaps_) {
dosemaps.emplace(p_dosemap.file_energy, HBHEDarkening::readDoseMap(p_dosemap.fp.fullPath()));
Comment on lines +68 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is stupid question (in that case sorry!), but why do you need to define a dosemaps_ private member, fill it in the constructor, and here define another object "reading the dose" from the filepath stored in dosemaps_?

Couldn't you just use HBHEDarkening::readDoseMap(filePath) directly in the constuctor (modifying the dosemaps_ definition to std::map<int, std::vector<std::vector<float>>>, of course) and just use it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Couldn't you just use HBHEDarkening::readDoseMap(filePath) directly in the constuctor (modifying the dosemaps_ definition to std::map<int, std::vector<std::vector<float>>>, of course) and just use it here?

I could, but not knowing how much memory that would take, I didn't want to. In this way the file is also read only if some other module in the job actually consumes HBHEDarkening.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok makes sense, thanks Matti!

}

return std::make_unique<HBHEDarkening>(pset_.getParameter<int>("ieta_shift"),
pset_.getParameter<double>("drdA"),
pset_.getParameter<double>("drdB"),
dosemaps,
years);
return std::make_unique<HBHEDarkening>(ieta_shift_, drdA_, drdB_, std::move(dosemaps), years_);
}

DEFINE_FWK_EVENTSETUP_SOURCE(HBHEDarkeningEP);
10 changes: 9 additions & 1 deletion CalibCalorimetry/HcalPlugins/src/HBHEDarkeningEP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,15 @@ class HBHEDarkeningEP : public edm::ESProducer, public edm::EventSetupRecordInte
edm::ValidityInterval&) override;

private:
const edm::ParameterSet& pset_;
struct Dosemap {
edm::FileInPath fp;
int file_energy;
};
std::vector<Dosemap> dosemaps_;
std::vector<HBHEDarkening::LumiYear> years_;
const double drdA_;
const double drdB_;
const int ieta_shift_;
};

#endif
4 changes: 2 additions & 2 deletions CondFormats/HcalObjects/interface/HBHEDarkening.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ class HBHEDarkening {
HBHEDarkening(int ieta_shift,
float drdA,
float drdB,
const std::map<int, std::vector<std::vector<float>>>& dosemaps,
const std::vector<LumiYear>& years);
std::map<int, std::vector<std::vector<float>>> dosemaps,
std::vector<LumiYear> years);
~HBHEDarkening() {}

//public accessors
Expand Down
6 changes: 3 additions & 3 deletions CondFormats/HcalObjects/src/HBHEDarkening.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
HBHEDarkening::HBHEDarkening(int ieta_shift,
float drdA,
float drdB,
const std::map<int, std::vector<std::vector<float>>>& dosemaps,
const std::vector<LumiYear>& years)
: ieta_shift_(ieta_shift), drdA_(drdA), drdB_(drdB), dosemaps_(dosemaps), years_(years) {
std::map<int, std::vector<std::vector<float>>> dosemaps,
std::vector<LumiYear> years)
: ieta_shift_(ieta_shift), drdA_(drdA), drdB_(drdB), dosemaps_(std::move(dosemaps)), years_(std::move(years)) {
//finish initializing years
std::sort(years_.begin(), years_.end());
//sum up int lumi
Expand Down