-
Notifications
You must be signed in to change notification settings - Fork 0
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
Try to update the code to Julia1.10 #36
Conversation
I was able to reproduce result (albeit for a half-resolution simulation) for all figures, with the exception of the pressure drag scaling. This is the result using data from our last run (with the old Oceananigans): and this is the same result with the new version of Oceananigans: and if I zoom out, this is where the points went: Which seems waaay to high. My guess is that some weird behavior got introduced in some intermediate Oceananigans version? It's gonna be hard to track this down though... |
Ah, okay, I think it may be that same pressure problem that was plaguing us before. I had made a hacky fix that worked that was on a separate branch. Then Greg and I made came up with a fix that we thought was going to actually solve the problem (and it apparently did) and it got merged into main. Maybe that PR didn't actually fix the issue... It's the only thing I can think of. |
For reference, this was my hacky fix: CliMA/Oceananigans.jl#3606 |
Hmm, I just noticed something strange though. The plot about the shear production rates appears to have significantly lower magnitudes than it previously did. The new plot (with the original color maps, although with half resolution) looks like this: while the original plot in the paper looks like this: The patterns look about the same, which can be evidenced by changing the color ranges: I wonder if this is coming from the post-processing. Every other result is the same, so there's no way that the shear production rate actually changed that much. It would have totally changed the whole energetics of the paper. |
Update/for the record: hi-res simulations with updated Julia and Oceananigans have started. |
So, post-processing the old (i.e. with Julia 1.9, old Oceananigans, etc.) output files now produces an output that's consistent with the current output from Julia 1.10: This is weird, because it's an order of magnitude lower than what the old output was: I don't know what to make of this. If I analyze the original time-averaged files, results are consistent with the lower magnitude result (see post here), so my guess is that at some point I changed the calculation somehow, but didn't update the plot in the paper? I think the change in result doesn't change the conclusions from our paper at all, since despite the lower magnitude, the patterns are the same, and therefore our figure analysis remains the exact same. But I don't know, I'd like to understand this a little better. At the same time, if I can't reproduce the old results, I don't see how I could do that (short of going back commits one by one and re-running everything for each, which may easily take weeks). @wenegrat your input is appreciated here. |
For reference, these are the lines that calculate the shear prod rates: submesoscale-headland/h01_energy_transfer.py Lines 191 to 198 in a5e4b6e
I don't see anything wrong with them. And if you check the commit history on those lines (with And that's pretty much it. The code that does the figure simply collects the time-averaged datasets from each simulation into one dataset:
Calculates the total and horizontal SPRates by summing over the appropriate indices: submesoscale-headland/hplot13_SPR_components.py Lines 36 to 37 in a5e4b6e
and then plots it: submesoscale-headland/hplot13_SPR_components.py Lines 50 to 56 in a5e4b6e
So really, I don't see where this difference may be coming from. Although in an old version of the code the color ranges are smaller, implying that the the data magnitude used to also be smaller (in line with the new results): fa00ba1#diff-f0237387477696db0d7345dda33771b0330f65158f5d3fe9907306442a7bb456R32 |
Also for the record, the change in the magnitude of the color ranges in the plot came from PR #37 However, I don't see any changes in that PR that would have actually have changed the magnitude of the SPR. The only thing of note is that a couple PR before that (https://github.com/tomchor/submesoscale-headland/pull/39/files) I changed I changed the target CFL from 0.9 to 0.95 (which is admittedly perilously close to the limit). However, I tested things out a fair amount before merging that PR and saw no changes in results. |
Remind me, I believe there was an issue where the prior shear production estimates also overestimated the total dissipation rates etc. such that we had the problem of figuring out where the energy was possibly going. Is that correct? If so the lower magnitude here might help explain some of that. |
That does sound familiar, although to be honest I had completely forgotten about it. I'll try to find a PR about it. If that's indeed true then it supports my assumption that these new (lower in magnitude) results are the correct ones. |
I didn't find the PR where we actually discuss it, but I did find a PR comment (namely, #26 (comment)) where we can see that the SPR is about an order of magnitude higher than it should be. So I'm, convinced these results (with lower magnitudes) are the correct ones. I really have no idea what happened there, but the good news is that the code is working with the up-to-date package versions. |
Finally happy with this one! Merging |
The code is very out of date since we didn't have Julia 1.10 on Casper until recently. This PR tries to update it to a recent version 🤞