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

Fix CustomSourceTime with times completely outside envelope range #1901

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

caseyflex
Copy link
Contributor

@caseyflex caseyflex commented Aug 13, 2024

Fixes this issue:
#1899

The behavior of CustomSourceTime is to interpolate for times within the envelope definition time range, and use nearest selection for times outside the envelope definition time range. When evaluating at a list of times which is completely outside the envelope definition time range, xarray.interp fails, because we pass it an empty list. This PR checks separately for this case, when no interpolation is needed.

@momchil-flex
Copy link
Collaborator

Thanks, the fix looks good, but don't we still want a validator to error or at least warn that (as far as I understand?) this source will basically inject nothing in the simulation?

@caseyflex
Copy link
Contributor Author

Thanks, the fix looks good, but don't we still want a validator to error or at least warn that (as far as I understand?) this source will basically inject nothing in the simulation?

It will actually inject something -- the usual behavior of CustomSourceTime is to just use the first or last value of the envelope provided for times outside of the envelope definition time range. But yeah, we should probably validate this because this is unlikely to be intentional.

@caseyflex caseyflex force-pushed the casey/fixcst branch 3 times, most recently from 90c5441 to 9db07f4 Compare August 13, 2024 16:57
@momchil-flex momchil-flex merged commit 526f04f into develop Aug 15, 2024
15 checks passed
@momchil-flex momchil-flex deleted the casey/fixcst branch August 15, 2024 11:32
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.

CustomSourceTime with no time values within the simulation time range
2 participants