Skip to content

Add time interpolation to mpc data #3559

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

eslickj
Copy link
Contributor

@eslickj eslickj commented Apr 11, 2025

Fixes None

Adds option to linearly interpolate from MPC TimeSeriesData.

Summary/Motivation:

Sometimes you have time series data that doesn't line up perfectly with model time indexes. @blnicho and @jsiirola, this is part of the recent MHE work.

Changes proposed in this PR:

  • Add some linear interpolation utility functions and use them to add interpolation capbility to get data and time points not in MPC TimeSeriesData

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@blnicho blnicho requested review from jsiirola, Robbybp and blnicho April 14, 2025 13:45
Copy link
Contributor

@Robbybp Robbybp left a comment

Choose a reason for hiding this comment

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

Thanks for the addition. This is definitely functionality we were missing. I think this would be better as it's own interpolate, or get_interpolated_data, method. Would that work for your application?

There is also the question of whether to support some form of interpolation in DynamicModelInterface. (E.g., in get_data_at_time, load_data, or dedicated methods for interpolation.) I'm fine with (and think I prefer) punting for now if you don't need this for your application.

Copy link
Contributor

@Robbybp Robbybp left a comment

Choose a reason for hiding this comment

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

Looks good once tests pass, thanks!

Comment on lines +46 to +47
"""Return an array floats interpolated from data at times_data in
time_set.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Return an array floats interpolated from data at times_data in
time_set.
"""Return an array of floats interpolated at the time points in time_set
from data defined at time_data.

return pos


def _get_interp_expr_vec(time_set, time_data, data, indexes=None):
Copy link
Member

Choose a reason for hiding this comment

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

indexes isn't described in the docstring

Copy link
Contributor

Choose a reason for hiding this comment

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

Is indexes used? If not, can it be removed?

Comment on lines +62 to +63
If data are Pyomo components, this will return Pyomo expressions
interpolation if data are floats, this will return floats.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If data are Pyomo components, this will return Pyomo expressions
interpolation if data are floats, this will return floats.
If data are Pyomo components, this will return Pyomo expressions.
If data are floats, this will return floats.

Copy link

codecov bot commented May 5, 2025

Codecov Report

Attention: Patch coverage is 94.73684% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.76%. Comparing base (32591c1) to head (8230e5b).

Files with missing lines Patch % Lines
pyomo/contrib/mpc/data/interpolation.py 94.44% 1 Missing ⚠️
pyomo/contrib/mpc/data/series_data.py 95.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3559   +/-   ##
=======================================
  Coverage   88.75%   88.76%           
=======================================
  Files         890      891    +1     
  Lines      102246   102284   +38     
=======================================
+ Hits        90749    90790   +41     
+ Misses      11497    11494    -3     
Flag Coverage Δ
builders 26.61% <13.15%> (-0.01%) ⬇️
default 84.87% <94.73%> (?)
expensive 34.00% <13.15%> (?)
linux 86.25% <94.73%> (-2.26%) ⬇️
linux_other 86.25% <94.73%> (+<0.01%) ⬆️
osx 82.84% <94.73%> (+0.16%) ⬆️
win 84.58% <94.73%> (+0.01%) ⬆️
win_other 84.58% <94.73%> (+0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

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.

6 participants