Open piyueh opened 5 years ago
Good catch, I am surprised we did not see that before when we converted to Python 3. Does this code even work fully in Python 2?
I guess there's another problem that requires to be fixed: line 191 should probably be patch = state.patch
instead of patch = solution.patch
.
After fixing the exec
problem and the patch
problem, the write
function can be successfully executed in both Python 2 and Python 3 without any runtime error. But I don't know if the resulting NetCDF files are correct or not. It seems the data in the NetCDF files do not follow any convention, so I'm not able to do a quick check with common post-processing software.
That's the essential reason why we have yet to implement full NetCDF support as there do not appear to be any AMR enabled codes that also write to NetCDF (or HDF for the most part as well).
I would personally be in favor of removing netcdf support completely. It seems almost certain that nobody has used it for years (if I'm wrong, please let me know). We have no tests for it and the code is quite a mess. @piyueh do you have a need to use this?
Our HDF support is actually quite good I think, and includes testing.
Nope, I don't need it. I was just playing around with the solution I/O.
I am fine removing this code.
We have started using netCDF more for our tsunami modeling projects, particularly now that NCEI has moved to providing topography/bathymetry DEMs only in netCDF format rather than ascii raster files. So we've recently added new Python utilities for dealing with these.
Also some of our clients now want our model output in netCDF format so I will need to work on this soon. Currently I have scripts that converts fgmax output (e.g. the maximum inundation over the full run) into netCDF outside of GeoClaw, but for making animations we will soon have to provide netCDF output of time frames.
Perhaps the code that is currently in pyclaw is out of date and should be rethought, I haven't really started looking at it. But I just want to point out that for geoscientific data netCDF is the standard and HDF alone is not enough.
I'll raise a separate issue to start a discussion about what netCDF format we should use for AMR output. Should I raise it on pyclaw or on geoclaw?
Given that we need to support it in PyClaw to handle plotting smoothly I think this is the right place to do it. I have still not been successful in finding a previous example as to how to store AMR grids in NetCDF formats. It's not that we cannot do it but that the format assumes a lot of things about static gridding.
@rjleveque Oh, I had no idea. So are you using the code currently in pyclaw/fileio/netcdf.py
? I was under the impression that that code wouldn't run. If it's working and you have an example, it would be good to add a test.
At line 173 and 175 of file
pyclaw/fileio/netcdf.py
, the use ofexec
to add new local variables doesn't seem to work in Python 3 (I'm using 3.7.1)Python 2.7.15, however, does not have problems on these two
exec
s (though Python 2.7.15 will encounter some other problems in the remaining code innetcdf.py
).This may have something to do with that, exec is a statement in Python 2 but is a function in Python 3. See the discussion here.