add self.mod_stable_weight to function _limit_Vp_change()#307
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #307 +/- ##
===========================================
- Coverage 79.02% 78.97% -0.06%
===========================================
Files 12 12
Lines 2727 2730 +3
===========================================
+ Hits 2155 2156 +1
- Misses 572 574 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
amoodie
left a comment
There was a problem hiding this comment.
Lucille, this looks great and is passing tests (including consistency checks). Two small requests:
- can you add the new array to the hooks documentation file and the table at the end here:
https://github.com/DeltaRCM/pyDeltaRCM/blob/develop/docs/source/reference/model/model_hooks.rst#array-hooks - can you fix the spacing around the
*onsed_toolslines 559 and 563. We use Python Black formatting for the project whenever possible
To your suggestion for the future, we kind of already have that for erosion with another modifying array. That might be the place to start, but might not cover all use cases, as you mention as well.
|
Andrew, thanks for the feedback and 2 new commits have been added in an attempt to address your requests. A few questions: -I added documentation for the basic structure of the use. I did not add details for the computation differs for deposition vs erosion. Is this acceptable or would you like more details? -I have never used Python Black before and manually made changes as opposed to using their checker. Is this acceptable? Changes made include spaces around * and adding new lines when character length increased above what seemed like normal. And thank you for pointing me to mod_erosion. I was aware of it but had not considered how it was implemented or how it might be used for the kind of scenario I described. |
amoodie
left a comment
There was a problem hiding this comment.
LGTM, thanks for making those changes. I'll merge as soon as the tests finish.
Currently written as a single array of ones to allow modification of _limit_Vp_change() computation in hooks. Future change idea separate into modify erosion and deposition weighting separately? Potential application example, addition of hard structures which can be deposited on but not eroded, oyster reefs. Not needed for my application but could be useful for others.