Skip to content
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

exec can not add new local variables in Python 3 #611

Open
piyueh opened this issue Nov 11, 2018 · 9 comments
Open

exec can not add new local variables in Python 3 #611

piyueh opened this issue Nov 11, 2018 · 9 comments

Comments

@piyueh
Copy link

piyueh commented Nov 11, 2018

At line 173 and 175 of file pyclaw/fileio/netcdf.py, the use of exec 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 execs (though Python 2.7.15 will encounter some other problems in the remaining code in netcdf.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.

@mandli
Copy link
Member

mandli commented Nov 11, 2018

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?

@piyueh
Copy link
Author

piyueh commented Nov 11, 2018

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.

@mandli
Copy link
Member

mandli commented Nov 11, 2018

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).

@ketch
Copy link
Member

ketch commented Nov 12, 2018

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.

@piyueh
Copy link
Author

piyueh commented Nov 12, 2018

Nope, I don't need it. I was just playing around with the solution I/O.

@mandli
Copy link
Member

mandli commented Nov 12, 2018

I am fine removing this code.

@rjleveque
Copy link
Member

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?

@mandli
Copy link
Member

mandli commented Nov 13, 2018

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.

@ketch
Copy link
Member

ketch commented Nov 13, 2018

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants