-
Notifications
You must be signed in to change notification settings - Fork 1
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
testing calc_rmax #48
Comments
Probably should drop this note here too: To reproduce your SlopeParam code @oceandie [which looks very much like my old Matlab ;) ] I did the following (see below). The large values seem to appear in the outer halo as they where included in the np.diff (your code only goes from the second index to the last but one) ... I also think I'd got the indices out of sync by 1 - but I'm not too sure ... I'll look again on Monday (and do an XR version of this). If I diff the the results from the following code with SlopeParam they match.
|
OK - I had to return quickly to look at the xr - just to clear my head! This does the same as the np, but I'm not too sure why the outer halo -> rmax[0,:], rmax[-1,:], rmax[:,0], rmax[:,-1] - still has values in. Diffing rmax[1:-1,1:-1] with the np and SlopeParam gives the same results @malmans2 perhaps it has something to do with
|
@oceandie if you're happy with these solutions, I'll push back to the branch |
Looks like pyroms is also using the "conservative approach" we discussed yesterday. That's already implemented so you just have to use the latest version of #40. There are two other minor sources of error at the boundaries and near coast, although I'm not convinced pyroms method is better (see notebook below)... Anyways, as @jdha mentioned we just have to mask land and boundaries. Here is the code to compare and reproduce pyroms implementation: https://nbviewer.jupyter.org/gist/malmans2/f6f1ee945fd0c5125b87cef275e86e2b |
Hi @jdha , thanks for looking at it! Glad the SlopeParam is similar to your matlab code, but as I specified in the doc of the function and in the description of the issue that is NOT my original function, I adapted it (very little) from pyROMS (if you are curious, see here ). So if I undersatnd correctly, there are two changes that are needed to replicate SlopeParam with calc_rmax_np:
|
I think that more importantly, pyROMS code and our "conservative approach" are consistent with the way NEMO implement the smoothing algorithm ... |
for calc_rmax_np, I re-wrote it above - and satisfies 1 & 2 |
Yes, yes I saw it ... was just a recap of the diffs ;) |
Regarding NaNs on the land: I think pyROMS and NEMO approaches are more correct - we don't want to include land points in the computation since
|
I found some inconsistencies in the way calc_rmax works. I've developed a quick python code for:
If we smooth the envelope with an rn_rmax threshold of 0.24, the following is the pring of the iterative smoothing:
python test_rmax_calc.py
Iter: 1 rmax: 1.0
Iter: 2 rmax: 0.9353992591835926
Iter: 3 rmax: 0.9187177073344386
Iter: 4 rmax: 0.8765126597150313
Iter: 5 rmax: 0.8185786808404448
Iter: 6 rmax: 0.7347370279088926
Iter: 7 rmax: 0.7347370279088925
Iter: 8 rmax: 0.6074021541852422
Iter: 9 rmax: 0.5717732932642535
Iter: 10 rmax: 0.4755012414806615
Iter: 11 rmax: 0.37086087298353765
Iter: 12 rmax: 0.24000000000000007
The rmax computed by the three codes are the following:
The text was updated successfully, but these errors were encountered: