-
Notifications
You must be signed in to change notification settings - Fork 349
feat(grid): add thickness method for all grid types #1138
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
Conversation
Includes option for passing array to calculate saturated thickness.
|
@jlarsen-usgs Added the proposed grid.thick() method for Mike Fienen. It also seems the grid.top_botm property was only appropriate for structured grids. Let me know what you think of the proposed changes. If it looks good I will edit the pull request and deprecate the thickness property on the ModflowDis class (we should probably do the same for the zcentroids property) and point users to the grid class. |
Codecov Report
@@ Coverage Diff @@
## develop #1138 +/- ##
=============================================
- Coverage 71.729% 71.605% -0.124%
=============================================
Files 225 225
Lines 51919 51967 +48
=============================================
- Hits 37241 37211 -30
- Misses 14678 14756 +78
|
jlarsen-usgs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Joe,
I've added some general comments and changes that will help remove the if/else statements from the code. The main being implementing a top_botm property method in the UnstructuredGrid class that overrides the base Grid class.
Add saturated_thick method to grid class to calculate saturated thickness. Can pass a mask value(s) to filter out cells. Added deprecation warning to ModflowDis for thickness property and postprocessing.get_saturated_thickness(). Revised notebooks and autotests to use grid.thick property and grid.saturated_thick method.
|
@jlarsen-usgs I implemented your suggestions. I modified them a bit because the unstructured was returning an array for the thickness instead of a vector. So thick return arrays with (nlay, nrow. ncol), (nlay, ncpl), and (nodes) shapes for structured, vertex, and unstructured grids. Also made thick a property and added a separate saturated_thick method for calculating the saturated thickness. |
|
@aleaf I was working on a generic When I was looking into replacing I found what looks to be a bug in the logic in unconf_thickness = np.where((hds - botm) > top, top, hds - botm)to unconf_thickness = np.where(hds > top, top - botm, hds - botm)I couldn't understand why a saturated thickness would be compared to the top elevation. Also added a check for heads less than the bottom of the cell (for MODFLOW-NWT). Added the following above the previous statement. hds = np.where(hds < botm, botm, hds) # for NWT when hds < botm |
Add saturated_thick method to grid class to calculate saturated thickness. Can pass a mask value(s) to filter out cells. Added deprecation warning to ModflowDis for thickness property and postprocessing.get_saturated_thickness(). Revised notebooks and autotests to use grid.thick property and grid.saturated_thick method.
|
@jlarsen-usgs You should also look at the t050 and t072 autotests (I think you are the one who worked on the parts of the autotests that I modified). I had to change some of the values in the assertions. The specific discharge values are different because I replace the postprocessing,get_saturated_thickness() method with the grid.saturated_thick() method. For cases where the head is above the top of the cell the postprocessing,get_saturated_thickness() method is calculating the pressure head not the aquifer thickness. So the fact that the vectors are changes does not surprise me but you should look at it to confirm this. |
Add saturated_thick method to grid class to calculate saturated thickness. Can pass a mask value(s) to filter out cells. Added deprecation warning to ModflowDis for thickness property and postprocessing.get_saturated_thickness(). Revised notebooks and autotests to use grid.thick property and grid.saturated_thick method. Fix bug in postprocessing.get_saturated_thickness() method.
|
@jdhughes-usgs I looked through the new changes for calculating saturated thickness and the associated tests and they make sense to me. The changes in vector values are what I would expect from changing from potentiometric surface to layer thickness when heads are higher than a given model cell's top elevation. |
Includes option for passing array to calculate saturated thickness.