Introduce a value range check on the numpy level #43
Conversation
source/aerobulk/flux.py
Outdated
| range_data = data | ||
| # check the absolute value if specified in range | ||
| if range[2]: | ||
| range_data = abs(range_data) |
There was a problem hiding this comment.
Do we need these lines to check for absolute value? Couldn't we just use a valid range of [-50,50] for both u_zu and v_zu that would achieve the same result?
There was a problem hiding this comment.
Yes, totally. And it will make the code wasier to read!
There was a problem hiding this comment.
Done. This is much nicer. Thanks for the suggestion.
| zt=2, | ||
| zu=10, | ||
| niter=6, | ||
| input_range_check=True, |
There was a problem hiding this comment.
Do we want this to be True by default? If this adds any extra time, then maybe we default to False?
There was a problem hiding this comment.
My thinking was that we should have the user friendly option by default. Think of a novice user that just naively puts in sst in C. Id much rather have them get a helpful error message.
BUT maybe we want to explicity say that the check costs performance in that warning?
There was a problem hiding this comment.
I added a warning that gets raise each time the range check is performed.
|
@paigem if you think those changes are sufficient, I think we can merge this after the tests pass. |
|
I think once this is done, we could release either 0.3.0 (since this has quite a few major new features) or 0.2.6. |
|
Ooh I don't even know where to start in making a release! But maybe that would be a good learning opportunity. Also, the CI tests didn't all pass. Looks like the |
Co-authored-by: Paige Martin <paigemar@umich.edu>
That would be the point 😜. Also its pretty easy (if things dont break haha). |
|
Thanks for fixing the CI! |
I have implemented a checking function on the numpy level that can be turned on/off. This function checks the 'shrunken' arguments for nans and values that are out of the allowed range. This should completely eliminate the need for propagating errors from the fortran level (#4, #11), since we are now catching eveything that would lead to a
STOPin fortran at the python level.