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

Feature/input structure #8

Merged
merged 152 commits into from
Jan 29, 2020

Conversation

cneyens
Copy link
Collaborator

@cneyens cneyens commented Sep 12, 2019

This PR contains a bunch of new functionality. I've written some vignettes to explain the major updates (they are not build but you should be able to knit them after pulling this PR and installing RMODFLOW again).

Input data structure:

  • rmf_arrays have been streamlined: subsetting, dimension labels, keeping attributes etc.
  • Similar to rmf_array, a rmf_list class can be created from data.frame-like objects to deal with MODFLOW list data
  • Functions for creating boundary conditions have changed. Now they're much less verbose (see vignette)
  • Reading output is more robust. A breaking change is that the bud keyword is replaced with cbc (for cell-by-cell flow data). The volumetric budget (i.e. as read from the listing file) is now assigned the bud keyword.
  • All input structures and formats can be read: binary, ASCII, fixed-format, free-format, fixed-format array headers, external or open/close arrays, ...
  • MODFLOW-NWT has been incorporated
  • The API has been updated for dealing with MODFLOW parameters which took a lot of work. Regardless, I advise against using those (see also vignette). Perhaps needs another look.
  • Confining beds should be handled correctly now, but warnings might appear. The problem with confining beds is that they do not appear in the dimensions of the DIS object (i.e. dis$nlay) but are explicitly represented by dis$botm and all calculations using this array.
  • Some important internal functions: rmfi_list_packages lists all packages in the MODFLOW-2005 family as well as RMODFLOW supported packages. Every time a new package is supported, it should be added to the pack_names and rmf_names vectors. These are used in the top-level functions.
  • Internal read and write functions are more robust.
  • Some new utilities: conversions between flow packages, rmf_calculate_thickness, rmf_gradient & rmf_darcy (need to check overlap with rmf_convert_cbc_to_darcy which is probably better).
  • Two new dependencies: abind & ggquiver (for vector plotting)

Plotting:

  • Additional plotting functions for boundary conditions & output
  • Added some new input arguments for existing plot functions
  • New type = 'vector' for plotting vector arrows. The smoothing option still needs some work (perhaps using interpolation like the contour type).

Top-level:

  • Top-level functions rmf_create(), rmf_write() and rmf_read() handle entire MODFLOW models in a robust way.
  • I've noticed that sometimes I get permission errors with rmf_write(). I think this has to do with my admin rights on the laptop since rmf_write() is just a fancy wrapper for all other rmf_write_* functions. It also doesn't always give an error, which makes it even stranger...

Probably forgetting a bunch of stuff

Some important notes:

  • My vision for this PR was that RMODFLOW could handle ANY type of MODFLOW-2005 input & output, which, at this point, it should (as long as the packages are supported).
  • As stated above, every time a new package is supported, the rmfi_list_packages function should be updated as well as the wiki
  • Note that the roxygen documentation still gives some warnings in the build. I've not added any examples yet.
  • Do you want to keep the exported deprecated function names ? Namespace is getting crowded... Similarly, do you want to keep the rmf_read_fhd & rmf_read_bhd functions? They're basically wrappers for rmf_read_hed with the binary argument set differently.

Up-next:
I have some print functions on another branch + some spatial conversion functions (not pushed yet) but I think that the more urgent addition should be tests (+ CI) seeing as the package is getting pretty big...

In the future, remind me not to make these PR's so big ;)

@cneyens
Copy link
Collaborator Author

cneyens commented Nov 7, 2019

Now also includes print functions (through commits b3e1954 d7609f9 )

@rogiersbart
Copy link
Owner

Hi Cas,

thanks for all this work.
Here's already some feedback on your questions above:

  • I think we can get rid of the deprecated functions now. We're still in experimental phase I would say, so no problem. I do think we (i.e. mostly you) are getting closer to a first major version, from which point on this would be something to be more careful with.
  • Concerning the rmf_read_fhd and rmf_read_bhd functions: I do want to keep consistency with ModelMuse as much as possible, so they should stay if you ask me (i.e. these are the file extensions used there).
  • Concerning the testing, let's discuss in RMODFLOW extension packages #10.

I'll try to have a deeper look into all of this. Not sure if I will add some commits here or just merge ... There's a lot to check out! :)

@rogiersbart
Copy link
Owner

Deprecated functions are gone now. Concerning item 2 above, see also #11.

@rogiersbart rogiersbart merged commit 2fec953 into rogiersbart:develop Jan 29, 2020
@cneyens cneyens deleted the feature/input-structure branch January 30, 2020 19:13
@cneyens cneyens mentioned this pull request Jan 30, 2020
@cneyens cneyens mentioned this pull request Mar 13, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants