-
Notifications
You must be signed in to change notification settings - Fork 5
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
Final tweaks to binDensity.py #19
Comments
actually it comes from the first time step, i.e. K=1 in plot/over/thick=1 ptopsigmap[K=1:20@ave] As you can see, the red line is ok. On 22/9/14 19:56, Paul J. Durack wrote:
Eric Guilyardi
|
ok I had a first go at points 4 and 6 below. You can start from this On 22/9/14 19:30, Paul J. Durack wrote:
Eric Guilyardi
|
Ok great, I've just pulled this across into my repo and will start where you finished up.. |
I'm guessing you've had trouble getting this to run as you redeclare the so/thetao variables as npy arrays, thereby breaking their cdms2 transient variable status and then leading to calls like |
Is there a reason why arrays are preallocated with 'NaN' values rather than valmask? We should really avoid using NaNs within code, cdms2 doesn't play nicely with them. |
Ok so my latest durack1@8e6d581 should (I've just kicked off a run for testing) solve problems 2 and 5. I'll still need your help with the mask issue (1) and will also have to get back to renaming the netcdf file variables (3), which will also probably lead me to addressing (4) as I learn what is going on. I've also replaced the https://github.com/durack1/Density_bining/blob/master/binDensity.py#L321-324 |
I was just thinking.. This mask issue could be due to different behaviours with NaNs from numpy 1.9.0 (mine) to 1.7.1 (yours).. And is a motivator to address some of the NaN declarations noted above |
Ok so seems with durack1@9d654dc I'm now getting this error, which only occurs into the second timestep of the analysis:
I've got a feeling that purging this is causing the issue: What else shouldn't be purged (or what code can we comment out)? |
yep, too violent a purge...
Eric Guilyardi
|
well, these arrays are used in a numpy interpolation below (npy.interp) On 24/9/14 03:09, Paul J. Durack wrote:
Eric Guilyardi
|
indeed ! because I cannot use the test mode, it takes a while to see On 24/9/14 01:43, Paul J. Durack wrote:
Eric Guilyardi
|
hum... why would this happen only for the first year ?
Eric Guilyardi
|
ok, my last commit (76dfac1) works. I changed drive_IPSL.py on crunchy to have the timeint='1,12' flag and that speeds up testing |
IPSL run using last commit:
|
just tested the compressed file done on crunchy and the mask issue for year 1 is gone. So it must indeed be due to the new bumpy. Let me know if replacing nan with valmask would work with npy.interp (but I still don't understand why it would have this specific behaviour...) |
@eguil I have a feeling that it's the NaNs that are causing this issue.. Although it highlights a numpy1.9.0 vs numpy1.7.1 behaviour change that we need to get to the bottom of. |
ok - I have now a version without NaNs that looks ok (commit ff5781d). Can you please try it with your version of numpy ? |
Actually, here is the latest version to use: 81d17f8 |
there was a bug in the thickness calculation. Now correct with commit 189a5ce |
Ok so you seem to have things working with b655466 - should I pull this across and run again? |
@eguil so I'm now starting to test across the suite, and have hit a problem with non-standard (lat/lon) grids:
https://github.com/durack1/Density_bining/blob/master/binDensity.py#L671 I might have to engage with @doutriaux1 to determine a way to work around this issue.. |
@durack1 if ingrid is a grid even regular lat/lon it wouldn't work I can't remember how to make a curv grid out of thin air, I'll look, but in the mean time if you're going to save to do:
replace the depth axes
and then
then on dy don't forget to add the coordinates attribute on dy and save the "bound variables" into the file. Then reading in the file back in you should be good to go, I will look for curvlinear calls as well and will post here when I find them. |
Hi Paul, your message on grids is a bit cryptic ! Not sure what you want me to do I was also thinking of writing out the annual mean 3D lat/lon/rhon on On 2/10/14 02:20, Charles wrote:
Eric Guilyardi
|
@eguil the issue here is that the I have some work to do to get things working across all the models, ACCESS1-0 was the first in the list to process, and this model is what led to the error above #19 (comment) Skype at 9am sounds fine, I'll try from the office when I get in.. |
This is what I see for current files:
|
@eguil I got sidetracked today and made little progress on this.. I'll get back to it tomorrow.. |
@durack1 - ok let me know when you made progress. We need to start to do On 3/10/14 07:23, Paul J. Durack wrote:
Eric Guilyardi
|
@eguil what's the z_s variable and is it needed? https://github.com/durack1/Density_bining/blob/master/binDensity.py#L520 I've been trying to figure out what the indexing is (it's a weird combo of boolean and -1's).. I've just commented it out and seems to be running across the models now.. Will see how far it gets, and also what the outputs look like tomoz.. Looks like it's got to the C*'s.. |
@durack1 z_s is the depth of the bin. The line you commented sets the max depth of each column to the highest density (index N_s). It is most of the time rewritten by the interpolation line below : |
@eguil there are some weird cases where the input data trip over your code, so think I've worked out most of these quirks.. I've managed to get it running across all models (alphabetically) up to MIROC4h.. And have just tweaked things to deal with that model, so assuming it runs across the remaining inputs we should have the code working.. I'm now rewriting the outputs to clean them up, and then will kick it off for the whole historical archive.. |
@durack1 ok let me know how it goes. Have you integrated my latest version of binDensity.py ? |
@eguil I've hit a problem with the code that is going to need your intervention to sort out - I'm still not completely following how you're getting the result. My repo durack1@0a01625 should have a working version that is returning all masked values - I'm not sure what I've tangled up.. If you use drive_density: I've also collapsed all the zonal means into a single variable (and done some renaming), however the data in the arrays is rubbish due to what I think is a mask propagation issue..? If you can get this back working, it's good to go (I think!?!). It could still be further optimized - converting all code to pure numpy and removing use of npy.ma when we can.. But I think a result is more important than optimization at this stage, so we can leave it as is.. The output files are not fully CF-compliant, but that again is another less important issue, as these files wont be redistributed.. Probably a good idea to skype about this early this week if you're able.. |
@durack1 ok I'll have a look tomorrow. We can skype tomorrow (Tuesday)
Eric Guilyardi
|
@eguil sounds good.. We're close.. I also checked out the CF-compliance of the output files (even though the data is rubbish at the moment) and it's not as bad as I thought, so once this masking issue is resolved should be good to start running across the archive and doing some science! 9am tomorrow sounds great.. The new output format looks like:
|
@durack1 can you remind me how to pull your version accross to my repo On 27/10/14 19:44, Paul J. Durack wrote:
Eric Guilyardi
|
@eguil I can issue a pull request.. But that will wipe out your changes since my last pull from your repo.. You could try: |
@eguil October 1st was my last merge/pull from your repo: https://github.com/durack1/Density_bining/commits/master?page=2 |
@durack1 but how can I merge the changes made from Oct 1 by you and by me ?
Eric Guilyardi
|
It'll require some intervention.. Here's what git recommends: Step 1: From your project repository, check out a new branch and test the changes.
Step 2: Merge the changes and update on GitHub.
|
@eguil if your 5 model test was still running you've probably found that it failed.. I've managed to make some headway toward getting this running, but am still struggling with it.. If you have another go pull my version across.. I've commented out your new variables as they're particularly problematic.. If you're able to get it running across the 5 test models then let me know.. If you achieve this I think we then have a functioning prototype and I'll kick this off on the whole archive.. |
@durack1 I am making progress. ACCESS and BNU are ok (although for some
Eric Guilyardi
|
@eguil I have code which should sort out EC-EARTH - it's at https://github.com/durack1/Density_bining/blob/master/binDensity.py#L515-520 |
@durack1 - ok I will try
Eric Guilyardi
|
@durack1 the bug on zonal means all equal to global only happens to the On 29/10/14 14:24, Paul J. Durack wrote:
Eric Guilyardi
|
@eguil I get the impression from continuing the harvest through the code today that numpy indexing is an unknown entity.. This link should provide some insight:
|
@durack1 can we do a skype at 9am/5pm today ? I am confused by why
Eric Guilyardi
|
Ok so example:
|
@eguil ok so try this, it works for me on the single EC-EARTH test case, will run again over 5 models:
|
@eguil we need to be careful with this masking stuff, as in the test case it works, but as you're indexing the data directly
|
@eguil I've just updated the block two comments up #19 (comment) this also required a gotcha check for MIROC4h as it also has 0/incorrectly specified missing data.. This now works as advertised for the 5 model test case, and appears to return reasonable values.. If you can pull that across and check your versions, once you've then given me the all clear I'll pull back your updated (confirmed and committed) code from your repo and start processing over the archive.. |
@eguil strike that I need to re-sync to your master.. My code was missing the MIROC4h mask fudge.. |
@durack1 nope. it was a memory error further down the code (when trying
[ Time stamp 01/11/2014 14:40:29 ] On 3/11/14 07:08, Paul J. Durack wrote:
Eric Guilyardi
|
@eguil it seems that MIROC4h has caused another memory error on crunchy and has caused the job to bomb.. I'll figure out which run fell over and start from a point which excludes MIROC4h, I'll do this tomorrow.. |
@eguil ok, so running again from MIROC5 onward, we should hopefully have the "full" archive complete by the end of the next week.. |
@durack1 ok great ! On 24/11/14 20:05, Paul J. Durack wrote:
Eric Guilyardi
|
@eguil FYI, it seems that we still have mask issues for MIROC5 as per below:
We'll need to sort this mask issue out during the rewrite of the function.. I've kicked things off after skipping all the MIROC5 simulations, so from |
So there are a few things to cleanup the code and get it running across all the historical simulations:
del(var) ; gc.collect()
reduce memory footprint? [more to do]==> current version on my github outputs 4x4D monthly variables on WOA grid
==> I think you said you needed this to make sure the regrid works fine for distorted grids (issue 4 resolved below) ?
Completed:
The text was updated successfully, but these errors were encountered: