Composition-dependent bedrock erosion parameters in fastscape - #7086
Composition-dependent bedrock erosion parameters in fastscape#7086Wang-yijun wants to merge 10 commits into
Conversation
…isivity to be depth-dependent
|
I think it would be useful if @Minerallo or @Djneu could review this PR. |
|
hi @Wang-yijun, thanks for implementing this! I'll look closer later, but I am wondering if it is worth splitting this into two pull requests, one for land and one for marine? If I remember correctly, all functionality needed to change land parameters using ASPECT composition is already in the mainline FastScape version, but for the marine we would also need to update the fastscape-fortran version before it can be included. Though, I suppose in the end the changes are very similar so maybe we should first update the fastscape-fortran version and then do these both at once. What do you think @anne-glerum? |
anne-glerum
left a comment
There was a problem hiding this comment.
Thank you for implementing this @Wang-yijun! I'm too tired to give a full review, but I spotted some things to address.
@Djneu It would indeed have been nice if this PR was split into two, because these are two separate functionalities being implemented.
|
I've addressed your comments and removed the marine diffusicity changes. I'll open another one for those and add a check for Fastscape version there. |
anne-glerum
left a comment
There was a problem hiding this comment.
Hi @Wang-yijun, thanks for splitting up the PR. I think some changes are necessary to the way the average erosion rates are computed on which points. There are also some comments to make parts more efficient. Finally, what do you think about keeping in the option to just specify one, constant value for both rates?
|
You can press "Resolve conversation" on each comment you have addressed, that helps keep the overview of what is still open. |
I implemented one Fastscape feature:
(The marine part of this PR will move to another new PR)