Skip to content

Add structure for moving report.gms to Python - #142

Open
patrickbrown4 wants to merge 18 commits into
mainfrom
pb/report
Open

Add structure for moving report.gms to Python#142
patrickbrown4 wants to merge 18 commits into
mainfrom
pb/report

Conversation

@patrickbrown4

Copy link
Copy Markdown
Contributor

Summary

Following in the vein of #61 and #139, this PR adds a structure for moving the calculations in report.gms to Python and uses it for a couple blocks of outputs as examples.

Technical details

Implementation notes

The proposed structure is:

  • Sets, parameters, and output variables necessary for output-processing calculations are added in report_data.csv (in reeds/core/terminus)
    • These get written to a new intermediate file, {case}/outputs/results.gdx
    • Output variables always include all attributes (level, marginal, upper/lower bound, scale)
  • The existing report_dump.py calls a new report_calcs.main(), which returns a dictionary of output parameters calculated in Python. These parameters then get written to {case}/outputs/{param}.csv and/or {case}/outputs/outputs.h5, as usual.
  • report_calcs.main() has some helper functions for common operations at the top, followed by blocks of calculations for the different types of outputs.
    • Only the transmission, CO2 storage, and interconnection queue outputs are currently included; additional sections can be moved over in batches in subsequent PRs.

Some notes given the current mix of standards:

  • Parameters that are moved from report.gms to report_calcs.py are listed in a block comment at the top of report.gms, so searching for them will turn up both scripts.
  • Parameters not calculated in report.gms will not show up in {case}/outputs/rep_{case}.gdx
    • But {case}/outputs/outputs.h5 is unchanged
  • It's possible that doing these calculations in Python may be slower than doing them in GAMS, particularly for big hourly outputs, but some notes on potential speedups are added as comments in report_calcs.py

Additional changes

A new reeds.results.diff_outputs() function has been added. This function works like gdxdiff but for outputs.h5 files, and can be used to do a global comparison of outputs between pairs of cases.

Validation, testing, and comparison report(s)

Zero change for the USA_defaults case: results-v20260708_mainK0_USA_defaults,v20260709_reportK0_USA_defaults.pptx

Checklist for author

Details to double-check

  • Charge code provided to reviewers
  • Included comparison reports for appropriate test cases
  • Code formatting standardized
  • Reusable functions used where possible instead of copy/pasted code

General information to guide review

  • Zero impact on results of default case
  • No large data file(s) added/modified
  • No substantive impact on runtime for full-US reference case
  • No substantive impact on folder size for full-US reference case
  • No change to process flow (runreeds.py, reeds/core/solve/solve.py)
  • No change to code organization
  • No change to package requirements (environment.yml or Project.toml)

Did you use LLM tools (chatbot or copilot) in the preparation of this PR? If so, describe how

No

(dfs['tran_flow_all_rep'] * g['hours'] / dfs['tran_cap_energy']).groupby(['r','rr','trtype','t']).sum()
/ g['hours'].sum()
)
## NOTE: We here assume that all solve years use the same total weighting for stress

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The method on main is off for years before the last year; see #146. This implementation fixes it and is self-consistent when stress periods are always weighted at 6 hours per solve year, but if #146 is fixed as suggested, we should instead use hours_h(allh,t) here to make it future-proof in case we switch to nonuniform stress period weights.

@patrickbrown4
patrickbrown4 requested a review from bsergi July 14, 2026 17:40

@bsergi bsergi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. I added one small fix to get the logging working again.

Do you have any concerns about memory usage as we add more ouputs here? We already read in the whole gdx so it might be fine but wasn't sure.

You noted that the outputs.h5 file is unchanged--is that saying that you ran the new diff_outputs() function and didn't come up with any differences between this branch and main?

Comment thread reeds/core/terminus/report_calcs.py Outdated
Comment thread reeds/results.py
CO2_TRANSPORT_INV_out(r,rr,t)$tmodel_new(t) = CO2_TRANSPORT_INV.l(r,rr,t) ;
CO2_SPURLINE_INV_out(r,cs,t)$[tmodel_new(t)$csfeas(cs)] = CO2_SPURLINE_INV.l(r,cs,t) ;

CO2_FLOW_out(r,rr,h,t)$[(ord(r) < ord(rr))$tmodel_new(t)] = CO2_FLOW.l(r,rr,h,t) + CO2_FLOW.l(rr,r,h,t) ;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be CO2_FLOW.l(r,rr,h,t) - CO2_FLOW.l(rr,r,h,t) ;? The sum is used in eq_co2_transport_caplimit which makes sense, but if the goal is to just get flows for r < rr region pairs but preserve direction then I would think the second term should be negative as is done with the tran_flow_rep calculation. Not sure who is the lead on CO2 these days but might be worth checking with them.

If this is a bug it might make sense to just flag this in an issue and address it in a separate PR since the goal of this one is just to do the reporting and have it match what we have. If we did want to address here I think we would just to agg='net' over in report_calcs.py.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess there is the net flow calculation below which is what I was expecting, so maybe this one is fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm not sure why it's here; the number of permutations doesn't really seem necessary, but because the net flow is also included, it seems intentional to have both. It might be used to identify hours where the pipeline capacity is binding. I figured I'd leave it for parity with the existing outputs, and it could maybe be cleaned up as part of #2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants