feat(par-hit): multi-parameter LQ calibration via params config schema - #47
Open
ggmarshall wants to merge 3 commits into
Open
feat(par-hit): multi-parameter LQ calibration via params config schema#47ggmarshall wants to merge 3 commits into
ggmarshall wants to merge 3 commits into
Conversation
run_lq_calibration now loops over config['params'] — one entry per LQ
parameter, each with its own lq_param (no longer hardcoded lq80),
energy_param and optional dt_param/cdf/suffix/plot_options — mirroring
the multi-parameter A/E schema. Results nest per entry under the lq
key, objects as {name: LQCal}, plots per name. Suffixed entries need
LQCal.calibrate suffix support (legend-exp/pygama#707); the kwarg is
only passed when a suffix is configured so unsuffixed entries keep
working with released pygama.
The par-geds-hit-lq entry builds its column list from all entries and
no longer double-nests the full object dict under the pickle's lq key
(it stores just the {name: LQCal} mapping).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ranch Pre-resolves the aoe.py divergence with main: brings the params-loop run_aoe_calibration from #42 into this lineage, combined with the use_log_pdf knob from this branch, and carries the schema fixes from #46 (entry require list, AoE_Uncorr spelling, conditional suffix kwarg). The entry point also stores just the {name: CalAoE} mapping under the pickle's aoe key instead of re-nesting the full object dict, matching the lq script. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the GEDS hit-level LQ and A/E calibrations to support the multi-parameter params config schema (multiple calibrations per run), including optional suffixing to avoid output-name collisions and updated result/object nesting.
Changes:
- Update
run_lq_calibrationandpar-geds-hit-lqto iterate overconfig["params"], supporting per-entrylq_param,energy_param, optionaldt_param/cdf/suffix/plot_options, and storing{name: LQCal}under the pickle’slqkey. - Update
run_aoe_calibrationandpar-geds-hit-aoeto use the sameparamsschema, preserveuse_log_pdf, and nest results/objects/plots per params entry. - Adjust docstrings and CLI-required config keys to match the new schema.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/legenddataflowscripts/par/geds/hit/lq.py | Convert LQ calibration to params-driven multi-calibration flow; update CLI inputs and outputs accordingly. |
| src/legenddataflowscripts/par/geds/hit/aoe.py | Convert A/E calibration to params-driven multi-calibration flow while retaining use_log_pdf; update CLI inputs and outputs accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+268
to
+272
| for name, param_config in configs["params"].items(): | ||
| require_config_keys( | ||
| param_config, | ||
| ["lq_param", "energy_param"], | ||
| f"lq calibration config params entry '{name}'", |
Comment on lines
+190
to
+194
| for name, param_config in config["params"].items(): | ||
| require_config_keys( | ||
| param_config, | ||
| ["current_param", "energy_param"], | ||
| f"aoe calibration config params entry '{name}'", |
Comment on lines
+236
to
+241
| dt_cut=param_config.get("dt_cut", None), | ||
| dt_param=param_config.get("dt_param", 3), | ||
| high_cut_val=param_config.get("high_cut_val", 3), | ||
| compt_bands_width=config.get("debug_mode", 20), | ||
| debug_mode=debug_mode | config.get("debug_mode", False), | ||
| **({"use_log_pdf": True} if use_log_pdf else {}), |
Comment on lines
+435
to
+445
| for param_config in kwarg_dict["params"].values(): | ||
| params.append(param_config["current_param"]) | ||
| params.append(param_config["energy_param"]) | ||
| if "dt_param" in param_config: | ||
| params.append(param_config["dt_param"]) | ||
| else: | ||
| params.append("dt_eff") | ||
| if "dt_cut" in param_config and param_config["dt_cut"] is not None: | ||
| cal_dict.update(param_config["dt_cut"]["cut"]) | ||
| params.append(param_config["dt_cut"]["out_param"]) | ||
| params = list(dict.fromkeys(params)) |
Missing current_param/lq_param/energy_param in a params entry now raises a clear error naming the entry instead of a bare KeyError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the LQ calibration to handle multiple LQ parameters, mirroring the multi-parameter A/E schema from #42:
run_lq_calibrationloops overconfig["params"]— one entry per LQ parameter, each with its ownlq_param(no longer hardcodedlq80),energy_param, and optionaldt_param/cdf/suffix/plot_options. Results nest per entry under thelqkey, pickled objects as{name: LQCal}, plots per name. Config shape:Suffixed entries require
LQCal.calibratesuffix support (legend-exp/pygama#707); the kwarg is only passed when a suffix is configured, so unsuffixed entries keep working with released pygama. Thepar-geds-hit-lqentry point builds its column list from all entries and now stores just the{name: LQCal}mapping under the pickle'slqkey instead of re-nesting the full object dict.The second commit pre-resolves the
aoe.pydivergence with main: it brings the params-looprun_aoe_calibrationfrom #42 into this lineage combined with theuse_log_pdfknob, and carries the schema fixes from #46.Stacked on #44 (
perf-par-dsp) — only the two commits on top are new.Test plan
Full suite (unit + skimmed-data integration hit chain, ecal→aoe→qc→lq): 71 passed locally, with the LQ config exercising an unsuffixed primary plus a suffixed duplicate entry end to end, and the aoe config migrated to the params schema. Companion legend-dataflow pht chain (partcal→aoe→lq→fast) also passes.
Per
AI_POLICY.md: developed with AI assistance (Claude); reviewed by the submitter.🤖 Generated with Claude Code