SCECcode / awp

AWP with topography
BSD 3-Clause "New" or "Revised" License
1 stars 2 forks source link

Keeping the orginal write_grid tool #9

Closed deanrockit closed 5 years ago

deanrockit commented 5 years ago

Zhifeng has already written a very nice code to accommodate the structure of UCVM, which is very handy. Due to the fact that we don't always deal with UCVM, it would be very helpful if we could still keep the original write_grid tool which simply reads in the topo file and outputs the coordinates of all the curvilinear grid. Maybe renaming the one Zhifeng wrote as write_grid_UCVM is an option?

ooreilly commented 5 years ago

@deanrockit I believe @hzfmer built the UCVM tool on top of the curvilinear grid writer tool. I like the idea of having two tools, but it would nice to avoid code duplication. Would it be possible to refactor the code so that the curvilinear grid part of the tool is one place, but used by both tools?

hzfmer commented 5 years ago

@deanrockit @ooreilly

Yes the tool was built on top of the previous grid writer tool. There are two ways to meet @deanrockit 's need.

  1. Create a dummy mesh file with the size of (nx, ny, mz). The output file _coordz.bin is what @deanrockit needs; while the other output file mesh.bin can be put away.
  2. I could revise the code to separate the curvilinear grid part, but it will be less efficient. Or we could add a flag to control whether to generate the property mesh.

Which way would work best?

ooreilly commented 5 years ago

Not sure I understand why it would be less efficient. But in that case, I prefer:

  1. add a flag to control whether to generate the property mesh or not I prefer this option because it can be confusing with tools that the generate output files that are not needed, and also the tool will run faster if doesn't have to generate extra output files.
deanrockit commented 5 years ago

@ooreilly @hzfmer in order to not duplicate the same thing, I would vote for option3. The original version only takes the topo file and the parameters associated with it, whereas @hzfmer 's version takes in more inputs. It should be possible to distinguish two different scenarios by simply counting number of input arguments. And I don't quite understand why it's less efficient either.

hzfmer commented 5 years ago

I just provided a version adopting option 3.

However, it doesn't distinguish the two scenarios by the number of input arguments. Instead, the number of inputs will be the same for either case. If the parameter mesh_out is 0, some inputs will not be used (though users have to give these input parameters, just dummy values).

The inefficiency is caused by the way the outputs are written: the coordinates are computed and written at each layer, but the properties are written in a chunk of nz layers. We don't store coordinates at all layers to save memory cost.

Anyway, I also prefer the option 3. We may change how to read the input parameters to distinguish two scenarios.

ooreilly commented 5 years ago

@hzfmer your solution is fine by me, but some more testing would be nice :)

ooreilly commented 5 years ago

Closed #10