Skip to content

Check if either of BayesianSimple / MarkovChainMC #1244

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

Open
nucleosynthesis wants to merge 1 commit into
base: main
Choose a base branch
from
+16 −4
Open

Check if either of BayesianSimple / MarkovChainMC #1244

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
2 changes: 1 addition & 1 deletion interface/Combine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class Combine {
bool mklimit(RooWorkspace *w, RooStats::ModelConfig *mc_s, RooStats::ModelConfig *mc_b, RooAbsData &data, double &limit, double &limitErr) ;

std::string parseRegex(std::string instr, const RooArgSet *nuisances, RooWorkspace *w) ;
void addDiscreteNuisances(RooWorkspace *);
bool addDiscreteNuisances(RooWorkspace *);
void addNuisances(const RooArgSet *);
void addFloatingParameters(const RooArgSet &);
void addPOI(const RooArgSet *);
Expand Down
18 changes: 15 additions & 3 deletions src/Combine.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,13 @@
if (freezeAllGlobalObs_ && mc_bonly && mc_bonly->GetGlobalObservables()) utils::setAllConstant(*mc_bonly->GetGlobalObservables(), true);

// Setup the CascadeMinimizer with discrete nuisances
addDiscreteNuisances(w);
bool wsHasDiscretes = addDiscreteNuisances(w);

// Add a check that we're not trying to use discrete profiling with Bayesian methods
if ((algo->name() == "BayesianSimple" || algo->name() == "MarkovChainMC") && wsHasDiscretes){
throw std::invalid_argument("Cannot currently use discrete parameters with Bayesian methods. Either remove discrete parameters or remove runtime def option ADD_DISCRETE_FALLBACK");

Check warning on line 811 in src/Combine.cc

View check run for this annotation

Codecov / codecov/patch

src/Combine.cc#L811 

Added line #L811 was not covered by tests
}
Comment on lines +807 to +812
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix clang-format issues flagged by CI.

The logic correctly prevents Bayesian methods from being used with discrete parameters. However, the CI pipeline reports clang-format failures in this block. Please run clang-format to fix the formatting.

🔧 Suggested formatting fix 
   // Setup the CascadeMinimizer with discrete nuisances
-  bool wsHasDiscretes = addDiscreteNuisances(w);
-
-  // Add a check that we're not trying to use discrete profiling with Bayesian methods 
-  if ((algo->name() == "BayesianSimple" || algo->name() == "MarkovChainMC") && wsHasDiscretes){
-      throw std::invalid_argument("Cannot currently use discrete parameters with Bayesian methods. Either remove discrete parameters or remove runtime def option ADD_DISCRETE_FALLBACK");
-  }
+  bool wsHasDiscretes = addDiscreteNuisances(w);

+  // Add a check that we're not trying to use discrete profiling with Bayesian methods
+  if ((algo->name() == "BayesianSimple" || algo->name() == "MarkovChainMC") && wsHasDiscretes) {
+    throw std::invalid_argument(
+        "Cannot currently use discrete parameters with Bayesian methods. Either remove discrete "
+        "parameters or remove runtime def option ADD_DISCRETE_FALLBACK");
+  }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/Combine.cc` around lines 807 - 812, The block around
addDiscreteNuisances(w) and the Bayesian check is failing clang-format; reformat
the if-statement and surrounding lines (the call to addDiscreteNuisances, the
wsHasDiscretes variable and the if that checks algo->name() == "BayesianSimple"
|| algo->name() == "MarkovChainMC") to match project style—run clang-format on
src/Combine.cc or manually adjust spacing/brace placement so the throw
std::invalid_argument line and the if condition adhere to the codebase's
formatting rules.


// and give him the regular nuisances too
addNuisances(nuisances);
addFloatingParameters(w->allVars());
Expand Down Expand Up @@ -1194,17 +1200,20 @@
if (! arg->isConstant()) (CascadeMinimizerGlobalConfigs::O().allFloatingParameters).add(*arg);
}
}
void Combine::addDiscreteNuisances(RooWorkspace *w){
bool Combine::addDiscreteNuisances(RooWorkspace *w){

// return value
bool hasDiscrete = false;
RooArgSet *discreteParameters = (RooArgSet*) w->genobj("discreteParams");

CascadeMinimizerGlobalConfigs::O().pdfCategories = RooArgList();
CascadeMinimizerGlobalConfigs::O().allRooMultiPdfParams = RooArgList();

// Check for the discrete parameters
if (discreteParameters != 0) {
for (RooAbsArg *arg : *discreteParameters) {
RooCategory *cat = dynamic_cast<RooCategory*>(arg);
if (cat && (!cat->isConstant() || runtimedef::get("ADD_DISCRETE_FALLBACK"))) {
hasDiscrete = true;
if (verbose){
//std::cout << "Adding discrete " << cat->GetName() << "\n";
CombineLogger::instance().log("Combine.cc",__LINE__,std::string(Form("Adding discrete %s ",cat->GetName())),__func__);
Expand All @@ -1220,6 +1229,7 @@
RooCategory *cat = dynamic_cast<RooCategory*>(arg);
if (! (std::string(cat->GetName()).find("pdfindex") != std::string::npos )) continue;
if (cat/* && !cat->isConstant()*/) {
hasDiscrete = true;

Check warning on line 1232 in src/Combine.cc

View check run for this annotation

Codecov / codecov/patch

src/Combine.cc#L1232 

Added line #L1232 was not covered by tests
if (verbose){
//std::cout << "Adding discrete " << cat->GetName() << "\n";
CombineLogger::instance().log("Combine.cc",__LINE__,std::string(Form("Adding discrete %s ",cat->GetName())),__func__);
Expand All @@ -1228,6 +1238,7 @@
}
}
}

// Now lets go through the list of parameters which are associated to this discrete nuisance
RooArgSet clients;
utils::getClients(CascadeMinimizerGlobalConfigs::O().pdfCategories,(w->allPdfs()),clients);
Expand All @@ -1241,6 +1252,7 @@
if (! (v->isConstant())) (CascadeMinimizerGlobalConfigs::O().allRooMultiPdfParams).add(*v) ;
}
}
return hasDiscrete;
}

template <class Var>
Expand Down
Loading