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

Jump to bottom

Add the variables for param NP to the datacard parsing and writing methods #308

Merged
merged 3 commits into from
Nov 10, 2023
Merged

Add the variables for param NP to the datacard parsing and writing methods #308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0b22e84
add param NP to the datacard parsing and writing methods
anigamova Oct 16, 2023
037364a
drafting a new method
anigamova Oct 17, 2023
0084c55
add the working version setting up param np to the datacard creation …
anigamova Oct 31, 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
2 changes: 1 addition & 1 deletion CombineTools/interface/CombineHarvester.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ class CombineHarvester {
std::vector<std::string> channel,
std::vector<std::string> procs,
ch::Categories bin, bool signal);

void AddSystVar(std::string const& name, double val, double err);
void AddSystFromProc(Process const& proc, std::string const& name,
std::string const& type, bool asymm, double val_u,
double val_d, std::string const& formula,
Expand Down
12 changes: 12 additions & 0 deletions CombineTools/src/CombineHarvester_Creation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,18 @@ void CombineHarvester::AddSystFromProc(Process const& proc,
systs_.push_back(sys);
}

void CombineHarvester::AddSystVar(std::string const& name,
double val, double err) {
Parameter * param = SetupRateParamVar(name,val);
param->set_val(val);
param->set_err_u(+1.0 *err);
param->set_err_d(-1.0 *err);
auto sys = std::make_shared<Systematic>();
sys->set_name(name);
sys->set_type("param");
systs_.push_back(sys);
}

void CombineHarvester::RenameSystematic(CombineHarvester &target, std::string const& old_name,
std::string const& new_name) {
for(unsigned i = 0; i<systs_.size(); ++i){
Expand Down
33 changes: 26 additions & 7 deletions CombineTools/src/CombineHarvester_Datacards.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,10 @@ int CombineHarvester::ParseDatacard(std::string const& filename,
start_nuisance_scan = true;
}
}

if (start_nuisance_scan && words[i].size() >= 4) {
if (boost::iequals(words[i][1], "param")) {
std::string param_name = words[i][0];
if (!params_.count(param_name))
params_[param_name] = std::make_shared<Parameter>(Parameter());
Parameter * param = params_.at(param_name).get();
param->set_name(param_name);
Parameter * param = SetupRateParamVar(param_name, boost::lexical_cast<double>(words[i][2]));
param->set_val(boost::lexical_cast<double>(words[i][2]));
std::size_t slash_pos = words[i][3].find("/");
if (slash_pos != words[i][3].npos) {
Expand All @@ -318,6 +314,10 @@ int CombineHarvester::ParseDatacard(std::string const& filename,
param->set_range_u(boost::lexical_cast<double>(tokens[2]));
}
}
auto sys = std::make_shared<Systematic>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a worry this could cause problems in other bits of code, e.g. PostFitShapes, where it tries to evaluate all the Systematic objects, and might not know what to do with a param type. It might also be fine, I'm not sure. I can see another way to fix this while avoiding creating a Systematic entry. It would be to add a new bool data member to the Parameter class that flags parameters that come from param lines in the datacard. Can make false by default, and only set it to true here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @ajgilbert, thanks a lot for the feedback,
I think in the PostFitFromWorkspace method the datacards are setup from the workspace, and the datacard is parsed only for the re-binning, and I checked that it works. In principle I'm fine with reverting it back and avoiding definition of Systematic entry, but I also think that it would be good to be able to setup the param nuisances from CH datacard creation workflow, so that you can define formula based rate parameters. I just pushed a couple of commits that do that. Let me know what you think

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok yes, I see the point, indeed it's useful if these can be created directly. Do you have any other commits to add? If not we can merge.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I do not have anything to add at this point, thanks!

sys->set_name(param_name);
sys->set_type("param");
systs_.push_back(sys);
continue; // skip the rest of this now
}
}
Expand Down Expand Up @@ -763,13 +763,16 @@ void CombineHarvester::WriteDatacard(std::string const& name,
auto bin_set = this->SetFromProcs(std::mem_fn(&ch::Process::bin));
auto proc_set = this->SetFromProcs(std::mem_fn(&ch::Process::process));
std::set<std::string> sys_set;
std::set<std::string> param_set;
std::set<std::string> rateparam_set;
this->ForEachSyst([&](ch::Systematic const* sys) {
if (sys->type() == "rateParam") {
rateparam_set.insert(sys->name());
} else {
sys_set.insert(sys->name());
}
else if (sys->type() == "param"){
param_set.insert(sys->name());
}
else sys_set.insert(sys->name());
});
txt_file << "imax " << bin_set.size()
<< " number of bins\n";
Expand Down Expand Up @@ -1016,6 +1019,21 @@ void CombineHarvester::WriteDatacard(std::string const& name,
}
}

for (auto par : params_) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if following changes above, could remove this part, and instead modify the block that writes param lines around L1241, to replace all_dependents_pars.count(p->name()) requirement by all_dependents_pars.count(p->name()) OR [new bool flag is set].

Parameter const* p = par.second.get();
if (p->err_d() != 0.0 && p->err_u() != 0.0 &&
param_set.count(p->name()) && !sys_set.count(p->name())) {
txt_file << format((format("%%-%is param %%g %%g") % sys_str_len).str()) %
p->name() % p->val() % ((p->err_u() - p->err_d()) / 2.0);
if (p->range_d() != std::numeric_limits<double>::lowest() &&
p->range_u() != std::numeric_limits<double>::max()) {
txt_file << format(" [%.4g,%.4g]") % p->range_d() % p->range_u();
}
txt_file << "\n";
}
}


for (auto const& sys : sys_set) {
std::vector<std::string> line(procs_.size() + 2);
line[0] = sys;
Expand All @@ -1024,6 +1042,7 @@ void CombineHarvester::WriteDatacard(std::string const& name,
bool seen_shape = false;
bool seen_shapeN2 = false;
bool seen_shapeU = false;
if (param_set.count(sys)) continue;
for (unsigned p = 0; p < procs_.size(); ++p) {
line[p + 2] = "-";
for (unsigned n = 0; n < proc_sys_map[p].size(); ++n) {
Expand Down