Skip to content
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.

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

Variable Cost Refactor Part 1: Function Data #331

Merged
merged 3 commits into from
Feb 22, 2024

Conversation

GabrielKS
Copy link
Contributor

The first part of a significant refactor of Sienna cost data structures. See NREL-Sienna/PowerSystems.jl#1056 for more details. This PR consists of a new type FunctionData, a new set of concrete subtype structs to represent various functions, and the necessary refactoring to use them rather than IS.PWL and such in other InfrastructureSystems data structures. I also performed some minor code deduplication.

IS, PSY, and PSB tests pass when using the appropriate branches of each plus PowerSystemsTestData:
Screenshot of passing tests

Copy link

codecov bot commented Feb 22, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (f74a857) 79.57% compared to head (dff57ce) 80.04%.
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #331      +/-   ##
==========================================
+ Coverage   79.57%   80.04%   +0.47%     
==========================================
  Files          53       54       +1     
  Lines        4244     4315      +71     
==========================================
+ Hits         3377     3454      +77     
+ Misses        867      861       -6     
Flag Coverage Δ
unittests 80.04% <94.82%> (+0.47%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/InfrastructureSystems.jl 80.00% <ø> (ø)
src/common.jl 100.00% <ø> (ø)
src/deterministic.jl 84.61% <100.00%> (+2.52%) ⬆️
src/probabilistic.jl 80.70% <100.00%> (+2.73%) ⬆️
src/scenarios.jl 84.31% <100.00%> (ø)
src/function_data.jl 98.59% <98.59%> (ø)
src/hdf5_time_series_storage.jl 91.31% <96.15%> (ø)
src/utils/utils.jl 62.73% <75.00%> (+0.03%) ⬆️

... and 3 files with indirect coverage changes

function transform_array_for_hdf(
data::SortedDict{Dates.DateTime, Vector{PiecewiseLinearPointData}},
)
quad_cost = hcat([get_points.(v) for v in values(data)]...)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this called quad_cost for a PWL data? I see that the name is prior to this PR but it seems incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does seem incorrect. I'll change it

@@ -481,7 +515,10 @@ function transform_array_for_hdf(data::SortedDict{Dates.DateTime, Vector{PWL}})
return t_quad_cost
end

function transform_array_for_hdf(data::Vector{PWL})
# TODO: old code here does not properly handle data with different numbers of points
Copy link
Member

Choose a reason for hiding this comment

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

This is ok. It was meant like that for proper storage in the HDF5 file

Copy link
Member

@jd-lara jd-lara left a comment

Choose a reason for hiding this comment

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

This PR looks ok to me, there is some pending work for the MarketBid Cost that requires scoping but we already decided to refactor that at a later stage.

@jd-lara jd-lara merged commit 7f0d636 into main Feb 22, 2024
8 of 10 checks passed
@jd-lara jd-lara deleted the gks/variable_cost_refactor branch May 29, 2024 04:58
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.

2 participants