Bc cvar updates#107
Conversation
There was a problem hiding this comment.
Here are some initial thoughts from a quick look. Happy to touch base when you get back to discuss any of these. Let me know when you've taken another pass here and I can review again.
One high-level comment: can you organize the PR summary text a bit more? The technical details section has a lot of information but is a bit difficult to follow. Also, I didn't quite understand your test output GSw_PRM_StressThreshold = country_-1_NCVAR_cvar failed. What does it mean to have a -1 threshold? Ultimately for the PR we'll also want a comparison of default case from cases.csv from both this branch and main to show there weren't any changes.
There was a problem hiding this comment.
revert the changes to this file before merging
| required = false | ||
| "--write_shortfall_samples" | ||
| help = "Write the sample-level shortfall" | ||
| help = "Write the sample-level shortfall (hourly, large)" |
There was a problem hiding this comment.
What are the large and small designations? I might suggest just "Write per-sample hourly shortfall by region" for this one and "Write per-sample total shortfall by region" for the one below.
| default = 0 | ||
| required = false | ||
| "--write_shortfall_samples_totals" | ||
| help = "Write per-sample total shortfall by region (small)" |
There was a problem hiding this comment.
this is total over the entire PRAS time period and not annual, right?
| _,_,_,_,energyunit = PRAS.get_params(sys) | ||
| alpha = Float64(args["cvar_alpha"]) | ||
| cvar_obj = PRAS.CVAR(energyunit, results["short_samples"], alpha) | ||
| @info(cvar_obj) |
There was a problem hiding this comment.
I think PRAS is setup so that the above info statements provide context (e.g., @info "$(PRAS.EUE(results["short"]))" results in EUE = 851000±5000 MWh/131400h MWh) being printed). Does this do something similar?
| end | ||
| ## Write it | ||
| sf = results["short_samples"] | ||
| region_names = sf.regions.names |
There was a problem hiding this comment.
the previous code filtered using sys.regions.names. I think this was because there are some pseudo regions for DC converter stations that don't have load and thus don't need to be included here, so we may want to continue to drop them (@patrickbrown4 might be able to weigh in on this one).
| return x.sort_values(ascending=False).iloc[:n_tail].mean() | ||
|
|
||
|
|
||
| def get_annual_cvar_stress_metric(case, t, stress_metric='NCVAR', iteration=0, alpha=0.95): |
There was a problem hiding this comment.
this function and the next one get pretty long, so it would be good to add comments throughout to provide some documentation on what they are doing.
| x = pd.Series(samples).dropna().astype(float) | ||
| if x.empty: | ||
| return np.nan | ||
| if not (0 <= alpha < 1): |
There was a problem hiding this comment.
Feels like this check might fit better in get_cvar_alpha. An even better approach might be to add a check before the ReEDS run which would avoid having runs get this far and then running into an error (best place for that would probably be here:
Line 222 in 1a8ab63
|
|
||
| def get_annual_cvar_stress_metric(case, t, stress_metric='NCVAR', iteration=0, alpha=0.95): | ||
| stress_metric = stress_metric.upper() | ||
| if stress_metric not in CVAR_METRICS: |
There was a problem hiding this comment.
The multi-metrics PR (#99) now drops the metric tag in the switch value; for example, GSw_PRM_StressThresholdNEUE now just has a value of transgrp_1_sum instead of transgrp_1_EUE_sum (see here). I think we could do the same in this PR and then you could drop the CVAR_METRICS and the check and just iterate over whichever switches are activated.
| ) | ||
| for criterion in sw[f'GSw_PRM_StressThreshold{metric}'].split('/'): | ||
| print(f"Evaluating GSw_PRM_StressThreshold {metric} with criterion: {criterion}") | ||
| stress_criteria = _evaluate_stress_threshold_criterion(stress_criteria, criterion, sw, t, iteration, dfenergy_r, stressperiods_this_iteration, |
There was a problem hiding this comment.
This line looks like just a formatting change--I'd suggest keeping as the original had it.
| return pd.concat(_metric, names=['level','metric','region']).rename(stress_metric) | ||
|
|
||
|
|
||
| def evaluate_cvar_target_check(sw, t, iteration=0, stress_metrics=None): |
There was a problem hiding this comment.
Much of this function, including the core check of failed = this_test.loc[this_test > threshold], seems to replicate behavior in _evaluate_stress_threshold_criterion. Do you think we could merge the CVAR treatment into that existing function and just have some control statements to turn off adding new stress periods for now when using CVAR metrics? It's possible having a second function makes the most sense here, but I worry a little bit about maintaining two similar functions.
Bumps the actions-non-breaking group with 2 updates: [actions/checkout](https://github.com/actions/checkout) and [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv). Updates `actions/checkout` from 6.0.2 to 6.0.3 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@de0fac2...df4cb1c) Updates `astral-sh/setup-uv` from 8.1.0 to 8.2.0 - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@0880764...fac544c) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 6.0.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions-non-breaking - dependency-name: astral-sh/setup-uv dependency-version: 8.2.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: actions-non-breaking ... Signed-off-by: dependabot[bot] <[email protected]>
…_actions/actions-non-breaking-0dffccaecd build(deps): bump the actions-non-breaking group with 2 updates
…alculations and differentiate LOLE and LOLD
…get_longest_events()
…tify new stress periods
…EDS-Model#123) * Remove instances of GSw_RegionResolution and agglevel variables * Delete hierarchy_from134.csv files * Clarify disaggregation docstrings and rename PCA_REG to legacy_ba * Cleanup * Undo rounding * Undo rounding * Revert changes * Rename PCA_REG to legacy_ba * bugfix: unitdata.csv already has 'r' column now * Misc. bugfixes/cleanup * Cleanup * Cleanup * bugfix: drop md5 and lat/lon columns from hierarchy in get_dfmap * Delete code writing hierarchy_with_res.csv * add hurdlereg for z90/hierarchy.csv; add z90 to allowed GSw_ZoneSet choices * z90 hierarchy.csv: fix El Paso NERC region * z90 hierarchy.csv: change '&' to 'and' in hurdlereg for GAMS * Update reeds/input_processing/copy_files.py Co-authored-by: Patrick Brown <[email protected]> * Remove 'regions' argument from main() * Update reeds/input_processing/copy_files.py Co-authored-by: Patrick Brown <[email protected]> * Delete commented out itlgrp constraint code * Remove remaining references to hierarchy_from134.csv * Delete 'aggreg' column * Delete modeled_regions.csv and references * Replace load allocation method switch compatibility check with warning in zones/README.MD * Add zoneset config and broaden definition of county-level zones * Add type indicators * Fix formatting * describe zoneset_config.yaml in inputs/zones/README.md * Remove now-inaccurate comment * Remove now-redundant special handling for county-level runs * Revert "Remove now-redundant special handling for county-level runs" This reverts commit a01d4be. * Remove now-redundant special handling for county-level runs --------- Co-authored-by: Patrick Brown <[email protected]>
…t_eue_events(); silence map_outagerate_new_stressperiods() if empty
…ull failures in plot_eue_events()
…EDS-Model#132) * bugfix: get_zonemap() output index is no longer nameless * Refactor workflow to create outputs with old format * Fix handling of kwargs in get_zonemap() * Remove unused row from level map * Bugfix --------- Co-authored-by: Kodi Obika <[email protected]> Co-authored-by: Kodi Obika <[email protected]>
Updated format of documentation on the setup page
…opower (ReEDS-Model#90) * Duplicate all changes from branch in private repo: - Add PSH data from ORNL (to replace data in repo) - Change relevant instances of `storage_duration(i)` with `storage_duration_m(i,v,r)` * Remove IHA dataset and replace with ORNL dataset * Update data source of cap_existing_psh.csv in sources.csv * Add storage_duration_m parameter to report_params.csv
Bumps the actions-major group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 6.0.3 to 7.0.0 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@df4cb1c...9c091bb) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions-major ... Signed-off-by: dependabot[bot] <[email protected]>
…_actions/actions-major-2217aebe03 build(deps): bump actions/checkout from 6.0.3 to 7.0.0 in the actions-major group
Summary
This PR adds CVAR/NCVAR reporting and target checks to the PRAS resource adequacy workflow.
This implementation builds on the
aa/metrics_extension/ MultiMetricRA workflow. Standard RA metrics are still used for stress-period selection, while CVAR/NCVAR are added as check-only metrics.Main idea is:
Standard stress-period metrics remain:
CVAR/NCVAR are evaluated separately after the standard RA metrics are written.
What changed
run_pras.jlAdded:
--cvar_alpha--write_shortfall_samples_totalsWhen
PRAS.ShortfallSamples()is available,run_pras.jlcan now write:PRAS_{t}i{iteration}-risk_metrics.csvPRAS_{t}i{iteration}-shortfall_totals_by_sample.h5risk_metrics.csvincludes CVAR/NCVAR-related PRAS risk metrics such as:shortfall_totals_by_sample.h5includes total shortfall by PRAS sample for USA and regions. These values are totals over the full PRAS resource adequacy period, not annual values.stress_periods.pyAdded CVAR/NCVAR handling with:
get_cvar_alpha()get_shortfall_totals_by_sample()_sample_cvar()get_annual_cvar_stress_metric()evaluate_cvar_target_check()CVAR/NCVAR are calculated by hierarchy level.
NCVAR is normalized by total PRAS load and reported in ppm.
CVAR is written as an annualized MWh value.
The standard
ra_metrics_{t}i{iteration}.csvoutput remains focused on the MultiMetricRA stress-period metrics. CVAR/NCVAR are written separately as:cvar_{t}i{iteration}.csvncvar_{t}i{iteration}.csvcvar_target_check_{t}i{iteration}.csvCVAR/NCVAR are intentionally kept out of the normal stress-period selection loop. They are only used for reporting and target checks.
ra_calcs.pyAdded logic so
write_shortfall_samples_totalsis turned on automatically when CVAR/NCVAR checks are requested.This output is also kept on when PRAS-informed PRM updates need sample-level shortfall totals.
runreeds.pyUpdated compatibility checks so
CVARandNCVARare allowed in:but treated as check-only metrics instead of standard stress-period metrics.
Switches / cases
Added or updated these switches:
GSw_PRM_CVARAlphaGSw_PRM_StressThresholdCVARGSw_PRM_StressThresholdNCVARUpdated the description of:
GSw_PRM_StressThresholdMetricsso it includes the standard MultiMetricRA stress-period metrics and the new check-only CVAR/NCVAR metrics.
The standard MultiMetricRA threshold format follows the
aa/metrics_extensionformat:CVAR/NCVAR threshold examples:
Example metric list:
The default remains unchanged unless CVAR/NCVAR are explicitly requested.
Testing
Added and ran a MultiMetricRA + CVAR/NCVAR test case instead of relying on the Pacific-only test case.
Test case:
The case produced the expected standard MultiMetricRA outputs:
ra_metrics_{t}i{iteration}.csveue_events_{t}i{iteration}.csvand the new CVAR/NCVAR check-only outputs:
cvar_{t}i{iteration}.csvncvar_{t}i{iteration}.csvcvar_target_check_{t}i{iteration}.csvExample result interpretation from the test case:
Because CVAR/NCVAR are check-only in this PR, a failed CVAR/NCVAR target check does not add stress periods or trigger a PRM update.
Checklist for author
Details to double-check
Charge code provided to reviewers
Included comparison reports for appropriate test cases
Documentation updated if necessary
If input data added/modified:
hourlize/resource.pywas rerun to regenerate the existing/prescribed VRE capacity dataCode formatting standardized
Reusable functions used where possible instead of copy/pasted code
General information to guide review
runreeds.py,reeds/core/solve/solve.py)environment.ymlorProject.toml)Did you use LLM tools (chatbot or copilot) in the preparation of this PR? If so, describe how
Used ChatGPT to help draft PR text and troubleshoot merge-conflict resolution. Code changes and test results were reviewed by the author.
Tag points of contact here if you would like additional review of the relevant parts of the model