Add Cost Alias Getters - Githubissues

NREL-Sienna / PowerSystems.jl

Data structures in Julia to enable power systems analysis. Part of the Scalable Integrated Infrastructure Planning Initiative at the National Renewable Energy Lab.

https://www.nrel.gov/analysis/sienna.html

BSD 3-Clause "New" or "Revised" License

299 stars 71 forks source link

Add Cost Alias Getters #1131

Closed GabrielKS closed 2 weeks ago

GabrielKS commented 2 weeks ago

The main goal here, discussed with @rodrigomha, is to allow a user of the ValueCurve cost aliases to get everything they need without unwrapping to the FunctionData. I also cleaned up some docs and tests that were outdated due to a VOM cost refactor. I didn't fix this failure, which I assume is due to changes elsewhere:

Adding @kdayday as a reviewer of these added semantics:

get_slopes(::PiecewisePointwiseCurve) calculates the slopes from the underlying pointwise FunctionData
get_slopes(::PiecewiseIncrementalCurve) fetches the slopes == the y-coordinates of the underlying step FunctionData
get_average_rates(::PiecewiseAverageCurve) fetches the average rates == the y-coordinates of the underlying step FunctionData

rodrigomha commented 2 weeks ago

I think we should have an interface directly to the CostCurve and FuelCurve (in addition to the interface to the value_curve), for example:

get_constant_term(cost_curve::CostCurve{LinearCurve}) = get_constant_term(get_value_curve(cost_curve))

What do you think @jd-lara @GabrielKS ?

GabrielKS commented 2 weeks ago

I think we should have an interface directly to the CostCurve and FuelCurve (in addition to the interface to the value_curve), for example:
get_constant_term(cost_curve::CostCurve{LinearCurve}) = get_constant_term(get_value_curve(cost_curve))
What do you think @jd-lara @GabrielKS ?

Mildly oppose, having so many methods only work on CostCurve if it is parameterized a certain way seems like bad design. Technically that is what I'm doing here too, but only because the user will be treating each cost alias as if it is a completely separate struct. But I could be convinced.

GabrielKS commented 2 weeks ago

Comments look good. Have you re-made to docs to see if these are showing up in the API or on the ValueCurve page? If not, maybe we can merge this into the docs branch when you're done where I'll also make that change to have a separate page for each valuecurve

No, haven't rebuilt the docs. Hopefully all the changes here should be independent of your efforts over there, should be minimal merge conflicts.