Skip to content

Refine plot plugin interface naming #303

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

Merged
njlane314 merged 1 commit into from
Sep 6, 2025
Merged

Refine plot plugin interface naming #303

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 docs/class_diagram.puml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ package plug #LightYellow {
}

interface IPlotPlugin {
+render()
+onPlot()
}

class PlotPluginManager {
Expand Down
2 changes: 1 addition & 1 deletion libplug/IPlotPlugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace analysis {
class IPlotPlugin {
public:
virtual ~IPlotPlugin() = default;
virtual void run(const AnalysisResult &result) = 0;
virtual void onPlot(const AnalysisResult &result) = 0;
};

}
Expand Down
4 changes: 2 additions & 2 deletions libplug/PlotPluginManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ class PlotPluginManager {
}
}

void run(const AnalysisResult &res) {
void notifyPlot(const AnalysisResult &res) {
for (auto &pl : plugins_)
pl->run(res);
pl->onPlot(res);
}

~PlotPluginManager() {
Expand Down
8 changes: 4 additions & 4 deletions libplug/plot/CutFlowPlotPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class CutFlowPlotPlugin : public IPlotPlugin {

explicit CutFlowPlotPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("plots") || !cfg.at("plots").is_array())
throw std::runtime_error("CutFlowPlotPlugin missing plots");
throw std::onPlottime_error("CutFlowPlotPlugin missing plots");
for (auto const &p : cfg.at("plots")) {
Comment on lines 30 to 33
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Sep 6, 2025

Choose a reason for hiding this comment

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

[P0] Replace accidental std::onPlottime_error rename

The global rename from run to onPlot also rewrote every std::runtime_error into std::onPlottime_error, which is not a standard exception type. Any translation unit that includes these constructors now fails to compile because std::onPlottime_error is undefined. Please revert these to std::runtime_error (or an application-specific type) in all affected plot plugin constructors.

Useful? React with 👍 / 👎.

PlotConfig pc;
pc.selection_rule = p.at("selection_rule").get<std::string>();
Expand All @@ -46,7 +46,7 @@ class CutFlowPlotPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &res) override {
void onPlot(const AnalysisResult &res) override {
StratifierRegistry strat_reg;
for (const auto &pc : plots_) {
auto signal_keys = this->fetchSignalKeys(strat_reg, pc.signal_group);
Expand All @@ -65,7 +65,7 @@ class CutFlowPlotPlugin : public IPlotPlugin {
metrics.eff_errors, metrics.purities, metrics.pur_errors, pc.output_directory,
pc.use_log_y);
plot.drawAndSave("pdf");
log::info("CutFlowPlotPlugin::run", pc.output_directory + "/" + pc.plot_name + "_" + pc.region + ".pdf");
log::info("CutFlowPlotPlugin::onPlot", pc.output_directory + "/" + pc.plot_name + "_" + pc.region + ".pdf");
}
}

Expand All @@ -74,7 +74,7 @@ class CutFlowPlotPlugin : public IPlotPlugin {
try {
return strat_reg.getSignalKeys(group);
} catch (const std::exception &e) {
log::error("CutFlowPlotPlugin::run", e.what());
log::error("CutFlowPlotPlugin::onPlot", e.what());
return std::nullopt;
}
}
Expand Down
6 changes: 3 additions & 3 deletions libplug/plot/EventDisplayPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class EventDisplayPlugin : public IPlotPlugin {

explicit EventDisplayPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("event_displays") || !cfg.at("event_displays").is_array()) {
throw std::runtime_error("EventDisplayPlugin missing event_displays");
throw std::onPlottime_error("EventDisplayPlugin missing event_displays");
}
SelectionRegistry sel_reg;
for (auto const &ed : cfg.at("event_displays")) {
Expand All @@ -52,9 +52,9 @@ class EventDisplayPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &) override {
void onPlot(const AnalysisResult &) override {
if (!loader_) {
log::error("EventDisplayPlugin::run", "No AnalysisDataLoader context provided");
log::error("EventDisplayPlugin::onPlot", "No AnalysisDataLoader context provided");
return;
}
for (auto const &cfg : configs_) {
Expand Down
6 changes: 3 additions & 3 deletions libplug/plot/FlashValidationPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class FlashValidationPlugin : public IPlotPlugin {

explicit FlashValidationPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("plots") || !cfg.at("plots").is_array())
throw std::runtime_error("FlashValidationPlugin missing plots");
throw std::onPlottime_error("FlashValidationPlugin missing plots");
for (auto const &p : cfg.at("plots")) {
PlotConfig pc;
pc.time_column = p.value("time_column", std::string("h_flash_time"));
Expand All @@ -47,9 +47,9 @@ class FlashValidationPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &) override {
void onPlot(const AnalysisResult &) override {
if (!loader_) {
log::error("FlashValidationPlugin::run", "No AnalysisDataLoader context provided");
log::error("FlashValidationPlugin::onPlot", "No AnalysisDataLoader context provided");
return;
}

Expand Down
8 changes: 4 additions & 4 deletions libplug/plot/OccupancyMatrixPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class OccupancyMatrixPlugin : public IPlotPlugin {

explicit OccupancyMatrixPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("occupancy_matrix_plots") || !cfg.at("occupancy_matrix_plots").is_array())
throw std::runtime_error("OccupancyMatrixPlugin missing occupancy_matrix_plots");
throw std::onPlottime_error("OccupancyMatrixPlugin missing occupancy_matrix_plots");
for (auto const &p : cfg.at("occupancy_matrix_plots")) {
PlotConfig pc;
pc.x_variable = p.at("x").get<std::string>();
Expand All @@ -51,17 +51,17 @@ class OccupancyMatrixPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &result) override {
void onPlot(const AnalysisResult &result) override {
if (!loader_) {
log::error("OccupancyMatrixPlugin::run", "No AnalysisDataLoader context provided");
log::error("OccupancyMatrixPlugin::onPlot", "No AnalysisDataLoader context provided");
return;
}
for (auto const &pc : plots_) {
RegionKey rkey{pc.region};
VariableKey x_key{pc.x_variable};
VariableKey y_key{pc.y_variable};
if (!result.hasResult(rkey, x_key) || !result.hasResult(rkey, y_key)) {
log::error("OccupancyMatrixPlugin::run", "Missing variables for region", rkey.str());
log::error("OccupancyMatrixPlugin::onPlot", "Missing variables for region", rkey.str());
continue;
}
PlotCatalog catalog(*loader_, 800, pc.output_directory);
Expand Down
8 changes: 4 additions & 4 deletions libplug/plot/RocCurvePlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class RocCurvePlugin : public IPlotPlugin {

explicit RocCurvePlugin(const nlohmann::json &cfg) {
if (!cfg.contains("roc_curves") || !cfg.at("roc_curves").is_array()) {
throw std::runtime_error("RocCurvePlugin missing roc_curves");
throw std::onPlottime_error("RocCurvePlugin missing roc_curves");
}
for (auto const &p : cfg.at("roc_curves")) {
PlotConfig pc;
Expand All @@ -56,9 +56,9 @@ class RocCurvePlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &) override {
void onPlot(const AnalysisResult &) override {
if (!loader_) {
log::error("RocCurvePlugin::run", "No AnalysisDataLoader context provided");
log::error("RocCurvePlugin::onPlot", "No AnalysisDataLoader context provided");
return;
}

Expand Down Expand Up @@ -88,7 +88,7 @@ class RocCurvePlugin : public IPlotPlugin {
try {
signal_keys = strat_reg.getSignalKeys(pc.signal_group);
} catch (const std::exception &e) {
log::error("RocCurvePlugin::run", e.what());
log::error("RocCurvePlugin::onPlot", e.what());
return false;
}

Expand Down
6 changes: 3 additions & 3 deletions libplug/plot/RunPeriodNormalizationPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class RunPeriodNormalizationPlugin : public IPlotPlugin {

explicit RunPeriodNormalizationPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("plots") || !cfg.at("plots").is_array())
throw std::runtime_error("RunPeriodNormalizationPlugin missing plots");
throw std::onPlottime_error("RunPeriodNormalizationPlugin missing plots");
for (auto const &p : cfg.at("plots")) {
PlotConfig pc;
pc.run_column = p.at("run_column").get<std::string>();
Expand All @@ -38,9 +38,9 @@ class RunPeriodNormalizationPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &) override {
void onPlot(const AnalysisResult &) override {
if (!loader_) {
log::error("RunPeriodNormalizationPlugin::run", "No AnalysisDataLoader context provided");
log::error("RunPeriodNormalizationPlugin::onPlot", "No AnalysisDataLoader context provided");
return;
}
for (auto const &pc : plots_) {
Expand Down
6 changes: 3 additions & 3 deletions libplug/plot/SlipStackingIntensityPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class SlipStackingIntensityPlugin : public IPlotPlugin {

explicit SlipStackingIntensityPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("plots") || !cfg.at("plots").is_array())
throw std::runtime_error("SlipStackingIntensityPlugin missing plots");
throw std::onPlottime_error("SlipStackingIntensityPlugin missing plots");
for (auto const &p : cfg.at("plots")) {
PlotConfig pc;
pc.run_column = p.at("run_column").get<std::string>();
Expand All @@ -39,9 +39,9 @@ class SlipStackingIntensityPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &) override {
void onPlot(const AnalysisResult &) override {
if (!loader_) {
log::error("SlipStackingIntensityPlugin::run", "No AnalysisDataLoader context provided");
log::error("SlipStackingIntensityPlugin::onPlot", "No AnalysisDataLoader context provided");
return;
}
for (auto const &pc : plots_) {
Expand Down
6 changes: 3 additions & 3 deletions libplug/plot/StackedHistogramPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class StackedHistogramPlugin : public IPlotPlugin {

explicit StackedHistogramPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("plots") || !cfg.at("plots").is_array())
throw std::runtime_error("StackedHistogramPlugin missing plots");
throw std::onPlottime_error("StackedHistogramPlugin missing plots");
for (auto const &p : cfg.at("plots")) {
PlotConfig pc;
pc.variable = p.at("variable").get<std::string>();
Expand All @@ -58,13 +58,13 @@ class StackedHistogramPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &result) override {
void onPlot(const AnalysisResult &result) override {
gSystem->mkdir("plots", true);
for (auto const &pc : plots_) {
RegionKey rkey{pc.region};
VariableKey vkey{pc.variable};
if (!result.hasResult(rkey, vkey)) {
log::error("StackedHistogramPlugin::run", "Could not find variable", vkey.str(), "in region",
log::error("StackedHistogramPlugin::onPlot", "Could not find variable", vkey.str(), "in region",
rkey.str());
continue;
}
Expand Down
6 changes: 3 additions & 3 deletions libplug/plot/SystematicBreakdownPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class SystematicBreakdownPlugin : public IPlotPlugin {

explicit SystematicBreakdownPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("plots") || !cfg.at("plots").is_array())
throw std::runtime_error("SystematicBreakdownPlugin missing plots");
throw std::onPlottime_error("SystematicBreakdownPlugin missing plots");
for (auto const &p : cfg.at("plots")) {
PlotConfig pc;
pc.variable = p.at("variable").get<std::string>();
Expand All @@ -34,13 +34,13 @@ class SystematicBreakdownPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &result) override {
void onPlot(const AnalysisResult &result) override {
gSystem->mkdir("plots", true);
for (auto const &pc : plots_) {
RegionKey rkey{pc.region};
VariableKey vkey{pc.variable};
if (!result.hasResult(rkey, vkey)) {
log::error("SystematicBreakdownPlugin::run", "Could not find variable", vkey.str(),
log::error("SystematicBreakdownPlugin::onPlot", "Could not find variable", vkey.str(),
"in region", rkey.str());
continue;
}
Expand Down
6 changes: 3 additions & 3 deletions libplug/plot/UnstackedHistogramPlugin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class UnstackedHistogramPlugin : public IPlotPlugin {

explicit UnstackedHistogramPlugin(const nlohmann::json &cfg) {
if (!cfg.contains("plots") || !cfg.at("plots").is_array())
throw std::runtime_error("UnstackedHistogramPlugin missing plots");
throw std::onPlottime_error("UnstackedHistogramPlugin missing plots");
for (auto const &p : cfg.at("plots")) {
PlotConfig pc;
pc.variable = p.at("variable").get<std::string>();
Expand All @@ -54,13 +54,13 @@ class UnstackedHistogramPlugin : public IPlotPlugin {
}
}

void run(const AnalysisResult &result) override {
void onPlot(const AnalysisResult &result) override {
gSystem->mkdir("plots", true);
for (auto const &pc : plots_) {
RegionKey rkey{pc.region};
VariableKey vkey{pc.variable};
if (!result.hasResult(rkey, vkey)) {
log::error("UnstackedHistogramPlugin::run", "Could not find variable", vkey.str(),
log::error("UnstackedHistogramPlugin::onPlot", "Could not find variable", vkey.str(),
"in region", rkey.str());
continue;
}
Expand Down
6 changes: 3 additions & 3 deletions plot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ static int runPlotting(const nlohmann::json &samples, const nlohmann::json &plot
for (auto &[beam, loader] : loaders) {
analysis::PlotPluginManager manager;
manager.loadPlugins(plotting, loader.get());
auto it = result_map.find(beam);
if (it != result_map.end())
manager.run(it->second);
auto it = result_map.find(beam);
if (it != result_map.end())
manager.notifyPlot(it->second);
}

analysis::log::info("plot::main", "Plotting routine terminated nominally.");
Expand Down