Skip to content

Add report generation package - #92

Open
sriharisundar wants to merge 17 commits into
mainfrom
ssh/PRAS_report
Open

Add report generation package#92
sriharisundar wants to merge 17 commits into
mainfrom
ssh/PRAS_report

Conversation

@sriharisundar

Copy link
Copy Markdown
Member

No description provided.

@codecov-commenter

codecov-commenter commented Oct 1, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.41877% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.97%. Comparing base (f49c56e) to head (cdf5e74).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
PRASReport.jl/src/writedb.jl 92.30% 14 Missing ⚠️
PRASCore.jl/src/Results/Shortfall.jl 0.00% 4 Missing ⚠️
PRASReport.jl/src/events.jl 96.72% 2 Missing ⚠️
PRASCore.jl/src/Results/Results.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #92      +/-   ##
==========================================
+ Coverage   84.14%   84.97%   +0.82%     
==========================================
  Files          45       49       +4     
  Lines        2491     2768     +277     
==========================================
+ Hits         2096     2352     +256     
- Misses        395      416      +21     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hsunnrel
hsunnrel requested a review from Copilot October 1, 2025 14:14

Copilot AI 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.

Pull Request Overview

This PR adds a new PRASReport.jl package to the PRAS ecosystem that generates interactive HTML reports for power system resource adequacy analysis results. The package extracts "events" (periods of energy shortfall above a threshold) from PRAS simulation results and creates visualizations using embedded databases and web technologies.

Key changes:

  • Implements event extraction and analysis from PRAS ShortfallResult objects
  • Creates interactive HTML reports with charts and tabular views using DuckDB WASM
  • Provides multiple input methods (results, system models, or file paths)

Reviewed Changes

Copilot reviewed 16 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
PRASReport.jl/src/ Core package implementation with event extraction, database operations, and HTML report generation
PRASReport.jl/test/ Test suite covering event extraction and report generation workflows
PRASReport.jl/examples/ Example usage script
PRASCore.jl/src/Results/ Extension to support new NEUE calculations needed by reporting
.github/workflows/ CI configuration for the new package

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

regions::Vector{String}
) where {N,L,T,E}

length(lole) != length(eue) != length(neue) != length(regions) &&

Copilot AI Oct 1, 2025

Copy link

Choose a reason for hiding this comment

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

This chained inequality comparison doesn't work as intended in Julia. It should be length(lole) != length(eue) || length(eue) != length(neue) || length(neue) != length(regions) to check that all vectors have equal length.

Suggested change
length(lole) != length(eue) != length(neue) != length(regions) &&
(length(lole) != length(eue) || length(eue) != length(neue) || length(neue) != length(regions)) &&

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Second this

Comment on lines +149 to +156
_write_db!(::ShortfallResult{N,L,T,E}, ::FlowResult{N,L,T,P},
threshold::Int, conn::DuckDB.Connection)

Write system and simulation parameters to the parameters table.
"""
function _write_db!(sf::ShortfallResult{N,L,T,E},
::FlowResult{N,L,T,P},
threshold::Int64,

Copilot AI Oct 1, 2025

Copy link

Choose a reason for hiding this comment

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

The second parameter is unused (anonymous with ::FlowResult). If the FlowResult is not needed for this function, consider removing it from the signature or document why it's required for the interface.

Suggested change
_write_db!(::ShortfallResult{N,L,T,E}, ::FlowResult{N,L,T,P},
threshold::Int, conn::DuckDB.Connection)
Write system and simulation parameters to the parameters table.
"""
function _write_db!(sf::ShortfallResult{N,L,T,E},
::FlowResult{N,L,T,P},
threshold::Int64,
_write_db!(sf::ShortfallResult{N,L,T,E},
threshold::Int, conn::DuckDB.Connection)
Write system and simulation parameters to the parameters table.
"""
function _write_db!(sf::ShortfallResult{N,L,T,E},
threshold::Int64,

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure why this was marked resolved.

@sriharisundar
sriharisundar marked this pull request as ready for review August 25, 2026 00:38
function NEUE(x::ShortfallResult{N,L,T,E}, r::AbstractString, t::ZonedDateTime) where {N,L,T,E}
i_r = findfirstunique(x.regions.names, r)
i_t = findfirstunique(x.timestamps, t)
return NEUE(div(MeanEstimate(x[r, t]..., x.nsamples),x.regions.load[i_r,i_t]/1e6))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Check if the load = 0 update ever made it through as tests might fail.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure if this should be here. We don't have this in any other repos.

CREATE TABLE events (
id INTEGER PRIMARY KEY DEFAULT nextval('eventid_sequence'),
name TEXT NOT NULL,
start_timestamp TIMESTAMP WITHOUT TIME ZONE NOT NULL,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this different from the start_timestamp in the systemsinfo table?

@@ -0,0 +1,95 @@
-- System and Simulation parameters

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

General comment here: DuckDB is incredbly fast at doing some of the calculatins we do for EUE, nEUE, etc. I know this might mean a full overhaul of the schema but something to think about.

regions::Vector{String}
) where {N,L,T,E}

length(lole) != length(eue) != length(neue) != length(regions) &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Second this

@@ -0,0 +1,141 @@
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

General comment: Is the API we want to go with? I don't have any comments on the code here, I'm not sure `create_pras_report(args, kwargs,) is what we like?

Comment on lines +149 to +156
_write_db!(::ShortfallResult{N,L,T,E}, ::FlowResult{N,L,T,P},
threshold::Int, conn::DuckDB.Connection)

Write system and simulation parameters to the parameters table.
"""
function _write_db!(sf::ShortfallResult{N,L,T,E},
::FlowResult{N,L,T,P},
threshold::Int64,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure why this was marked resolved.

threshold::Int=0,
title::String="Resource Adequacy Report")

base64_db = _get_base64_db((system_path,);threshold=threshold,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why can't we load the system and call the function above?

threshold=0,
samples=nothing, seed=nothing) where {N,L,T,P,E}

if isnothing(conn)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This gets called from _get_base64_db. I guess I don't understand why conn would ever be Nothing?

events = get_events(sf,threshold)

# Write events to database (events, system metrics, regional metrics)
foreach(event -> _write_db!(event,conn), events)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you use the appender API here?

@scdhulipala

Copy link
Copy Markdown

Couple of other comments:

  1. I feel like the embedding of the file directly into the HTML needs to be checked for scalability.
  2. Based on the tests, we might want to take a two file approach (.duckdb + .html)?
  3. Probably also include a caveat about how the report cannot be viewed offline maybe?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants