Skip to content

Fix extrapolation #46

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

Merged
merged 3 commits into from
Apr 21, 2025
Merged

Fix extrapolation #46

merged 3 commits into from
Apr 21, 2025

Conversation

MetinSa
Copy link
Collaborator

@MetinSa MetinSa commented Apr 20, 2025

Background

Extrapolation is not working at the moment as pointed out in #42.

Solution

Implement the suggestions from #45 and add new tests covering extrapolation

@MetinSa MetinSa requested a review from Copilot April 20, 2025 19:38
@MetinSa MetinSa self-assigned this Apr 20, 2025
@MetinSa MetinSa linked an issue Apr 20, 2025 that may be closed by this pull request
Copy link

@Copilot Copilot AI left a comment

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 addresses the extrapolation issue by introducing a new bounds_error parameter into the interpolation functions and propagating it through the model and test pipelines.

  • Added the bounds_error parameter to functions in unpack_model.py and model.py
  • Fixed a spelling error in variable naming from "paramameter" to "parameter"
  • Updated test strategies to randomly configure extrapolation via the new parameter

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
zodipy/unpack_model.py Propagated the new bounds_error parameter, applied it consistently in interpolation calls, and fixed a typo.
zodipy/model.py Added bounds_error handling when initializing the model and invoking the unpack function.
tests/strategies.py Modified test inputs to account for extrapolation behavior, ensuring test coverage for new extrapolation scenarios.

@MetinSa MetinSa requested a review from hermda02 April 20, 2025 19:45
@MetinSa
Copy link
Collaborator Author

MetinSa commented Apr 20, 2025

Hi @asabyr! Sorry for taking some time before attempt to fix the issue. I have implemented your suggestion in this PR and from some quick tests the extrapolation should be working. Let me know if anything is missing!

@MetinSa MetinSa marked this pull request as ready for review April 20, 2025 19:49
@MetinSa MetinSa linked an issue Apr 20, 2025 that may be closed by this pull request
Copy link
Collaborator

@hermda02 hermda02 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

@hermda02 hermda02 merged commit 1e4747f into main Apr 21, 2025
10 checks passed
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.

extrapolation question The extrapolation does not work
2 participants