-
Notifications
You must be signed in to change notification settings - Fork 37
Create PiecewiseLinearData conversion functions #512
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
Create PiecewiseLinearData conversion functions #512
Conversation
…pData Add bidirectional conversion capabilities: - PiecewiseStepData(::PiecewiseLinearData): converts by computing slopes (derivative) - PiecewiseLinearData(::PiecewiseStepData, initial_y): converts by running sum (integral) - Corresponding Base.convert methods for both directions The initial_y parameter allows preserving the y-offset when converting back from step data to linear data.
…onversions Test coverage includes: - Basic conversion from PiecewiseLinearData to PiecewiseStepData - Conversion from PiecewiseStepData to PiecewiseLinearData with default/custom initial_y - Base.convert methods for both directions - Round-trip conversion preserving data with initial_y parameter - Slope computation verification - Domain preservation
|
|
||
| # Test that slopes are correctly computed for non-trivial case | ||
| linear_data = IS.PiecewiseLinearData([ | ||
| (0.0, 0.0), (1.0, 1.0), (2.0, 4.0), (3.0, 6.0), (5.0, 10.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[JuliaFormatter] reported by reviewdog 🐶
| (0.0, 0.0), (1.0, 1.0), (2.0, 4.0), (3.0, 6.0), (5.0, 10.0) | |
| (0.0, 0.0), (1.0, 1.0), (2.0, 4.0), (3.0, 6.0), (5.0, 10.0), |
GabrielKS
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This functionality already exists one level up, i.e., you can already convert an InputOutputCurve that contains a PiecewiseLinearData to an IncrementalCurve that contains a PiecewiseStepData.
I believe that that's the more natural place to put this functionality, because a PiecewiseStepData does not inherently have any relationship to a PiecewiseLinearData, the derivative/integral relationship is only because these are often used to represent IOCs/ICs.
I would be very open to considering extensions of the ValueCurve interface to make those existing conversions easier to call, they are somewhat deliberately hard to call right now because of @jd-lara's expressed aversion to these conversions.
|
@GabrielKS I am trying to figure out a reasonable set of utilities to handle the need for conversions mostly for internal handling in PSI. I don't think we should do that inside of the modeling library and given the need to have some utility to clean datasets I am revising my position. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #512 +/- ##
==========================================
+ Coverage 79.10% 80.32% +1.22%
==========================================
Files 71 71
Lines 6192 6115 -77
==========================================
+ Hits 4898 4912 +14
+ Misses 1294 1203 -91
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
That makes sense — I think we should still probably do this only at the |
|
closing after @GabrielKS comments. |

…pData
Add bidirectional conversion capabilities:
The initial_y parameter allows preserving the y-offset when converting back from step data to linear data.