Added ability to convert VIC 4.2 ascii state files to VIC 5.0 netcdf

[tbohn]

Added ability to convert ascii state files to netcdf.

Addresses issue #35.

[tbohn]

Note: I plan on updating the code in this PR to make it consistent with development I'm currently doing in the VIC code. Some variable names will change and the set of coordinate variables in the output state file will change.

To see example ascii parameter and state files, and the corresponding netcdf parameter and state files produced by this version of tonic, please see:

meter:/raid/tbohn/VIC/test/data/NAM.tonic_test/params:

• global.try01 - this is the VIC run that created the ascii state file
• soil/veg/lake parameter files listed in global.try01
• params.try01.nc - netcdf parameter file output by tonic
• state.try01.nc - netcdf state file output by tonic

meter:/raid/tbohn/VIC/test/data/NAM.tonic_test/state:

• state.try01_19991231 - ascii state file output by VIC

[tbohn]

@jhamman - I see that tonic's write_netcdf() function names the coordinate dimensions 'ni' (for latitude) and 'nj' for longitude in the case of 2d coordinate variables. In this case, the order of the geographic part of the list of dimensions (in the definitions of other variables) is ('nj', 'ni',). Meanwhile, for 1d coordinate variables, tonic names them 'lat' and 'lon' and the order is ('lat', 'lon', ), which is the reverse of the 2d case. Any special reason for this?

VIC (in vic_store()) writes the dimension names as 'ni' and 'nj', and their order is (ny, nx) using the C convention, which is the reverse order of the python convention, so in python, they would be (nx, ny), with y corresponding to latitude and ni, and x corresponding to longitude and nj. So VIC appears to use the same convention as tonic's 2d case. This appears to be true even for VIC's 1d case.

Can I change tonic so that the dimensions are always named 'ni' and 'nj', and always appear in the order ('nj', 'ni', ) regardless of whether they are 1d or 2d?

[jhamman]

@tbohn -

The domain dimension convention should be:

1D Coordinates - use lat and lon for both dimension and coordinate names. (see http://cfconventions.org/Data/cf-conventions/cf-conventions-1.6/build/cf-conventions.html#idp5553648) 2D Coordinates - use dimension names that are meaningful (nj, and ni fit) and use 2d coordinate variables (see http://cfconventions.org/Data/cf-conventions/cf-conventions-1.6/build/cf-conventions.html#idp5559280).

I'm not exactly sure what you mean by "C convention" vs. "Python convention" since they should be the same. If we need to switch the dimension order around in tonic or vic_store, that is fine.

[tbohn]

Re: C vs Python - never mind. I dimly recall something about the C and Fortran netcdf libraries having opposite array index ordering conventions, but it's irrelevant. I'll just make sure that the arrays coming out of tonic are indexed the same way that VIC expects them to be indexed.

Re: coordinate dimensions and variables: OK, I'll follow the CF convention. I got confused by the 'lats' and 'lons' variables, which actually came from the soil parameter file. Tonic has been outputting both 'lat' and 'lon' 1d variables and the 'lats' and 'lons' 2d variables. Doing so certainly covers our bases, but is that really necessary? Why not just always output 2d coordinate variables called 'lat' and 'lon' (computed via grid params so that they cover cells outside the basin or land mask)? Why output the 'lats' and 'lons' variables from the soil param file (which were the inputs to calc_grid anyway, and therefore redundant)?

[tbohn]

(I just now read @jhamman 's comments on https://github.com/UW-Hydro/VIC/issues/367, in which some of my questions here are answered).

[tbohn]

Sorry to be such a nitpicker, but for 2d, tonic appears to be inconsistent with vic and CF convention. Here's the code for the 2d case: else: f.createDimension('nj', target_grid[XVAR].shape[0]) f.createDimension('ni', target_grid[YVAR].shape[1]) dims2 = ('nj', 'ni', ) coordinates = "{0} {1}".format(XVAR, YVAR)

In vic and the cf convention, i = x (= lon in a regular geographic grid), and j = y (= lat). I think in this case, if vic attempted to read the ni and nj dimensions from the file written by tonic, the grid would be transposed. I'll fix it in tonic.

[tbohn]

(and by 'fix', I mean I will swap i and j in the two createDimension calls; everything else will stay the same)

[yixinmao]

What is the direct input to this script (I assume it's a config file or something like that)? How should the utility be used exactly? I think it'd be helpful to add an example input config file together with some explanations (maybe in the "examples" directory) as well as an example of running command so that it's more user friendly.

[tbohn]

@yixinmao - to answer your question, ideally the script will take a config file as an input since there are now many input arguments. However, I believe that if you start a python session and import make_grid from grid_params, you might be able to set the values of the input arguments as environment variables. I haven't tried that yet. I just typed in every single one of them.

But I agree, I need to write some documentation at least explaining input options and the suffixes on my input files, and showing the option settings that I used. I'll do that today so that you can get started running tonic more quickly...

[tbohn]

@jhamman @yixinmao @dgergel - I created a readme.txt (meter:/raid/tbohn/VIC/test/data/NAM.tonic_test/params/readme.txt) that lists the input arguments I used to run make_grid() and also explains which options must be used with each of the various parameter files in the directory (in case you'd like to run tonic to test other option combinations).

I'll work on updating the code today and tomorrow (also working on resolving merge conflicts in the corresponding VIC PR for state file i/o). But this gives you something you can test out now, if you would like.

[tbohn]

OK, I've updated the code in response to everyone's comments, and this is ready for another review.

[tbohn]

@jhamman I'm finally working with tonic again, a year later, and I see that this whole line of development that I did never got merged - it's still just hanging there. What do we need to do to get this merged?

It looks like I prepared an update in response to all of the comments raised, but then nothing else has happened. I also see that development has occurred since then, and I'll need to resolve merge conflicts to get this ready for a new pull request.

Any objections to my resolving conflicts and submitting a pull request?

And which branch should I shoot for? I see there's a feature/vic42 branch now. Should this be merged into both vic42 and master?

[jhamman]

@tbohn - can you resolve the conflicts here then this can go in?

I'll leave it up to you and the other @UW-Hydro/vic-admins to decide where this should go (master vs. 42)...

[tbohn]

@UW-Hydro/vic-admins: I think I should merge this into feature/vic42 rather than master, since this is VIC 4.2-specific. Then, we can look into porting it into VIC 5.0.

I have a question about one of the conflicts that I'm trying to merge. feature/vic42/grid_params.py, function grid_params() has an argument of lake_dict=None. In my branch, there is no default for lake_dict. I'm happy to add the lake_dict=None just to eliminate the conflict. However, I see some inconsistencies throughout the code for other similar arguments, with some having default of None, some having default of False, and some having no default.

For now, I'll just resolve the conflict and make a new pull request. But should I open an issue for the inconsistent defaults?

[bartnijssen]

It'd be good to clarify in the name of the PR that this will convert VIC 4.2 ASCII state files into VIC 5.0 netcdf state files. It's not generic at this time (as @tbohn already points out).

[tbohn]

I squashed Test 1-5 of .travis.yml and then did a git push --force origin.

[tbohn]

Just a note - since issue #35 was modified last year to only pertain to the ascii -> netcdf direction (the opposite direction was split out into issue #44 ), accepting this PR will allow us to close issue #35.

[tbohn]

Thanks @dgergel you're right, there appears to be a lot of stuff in here that I didn't do. I don't know how those changes got in - I might have accidentally merged some stuff from feature/vic42 into here. Or something else.

The only changes I made were to grid_params.py and possibly vic_util.

Maybe I should close this PR and redo it cleanly?

[dgergel]

@tbohn yes I think it needs to be redone so the PR only has the code changes that you made. There's also a bunch of code re-formatting stuff in here that looks like it does need to be reformatted but should go into a separate PR. While you're doing that, it would also be helpful at the beginning of the PR to have a detailed list of changes that are going into it, e.g. new scripts and what they do. We've been stricter about that lately for VIC development and it's very helpful (e.g. it reminds you to run a format-checker before requesting code reviews)

[tbohn]

What do you mean by format checker - uncrustify? And these format problems, are they in grid_params.py or the other stuff that's not supposed to be in my PR?

[dgergel]

Yep, uncrustify. I meant that there are formatting changes in the PR that look like they need to be made (e.g. lines are too long) but these changes should be made in a different PR because they aren't specific to what you did. We just probably haven't run uncrustify on tonic for a while is my guess.

[tbohn]

OK, I'm closing this and will open a new PR shortly.