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

New DimensionlessDischarge Component #1377

Merged
merged 66 commits into from
Apr 14, 2022
Merged

Conversation

slundell123
Copy link
Contributor

This PR adds a new component and tests for a dimensionless discharge debris flow calculator. The DimensionlessDischarge component calculates the dimensionless discharge value, debris flow threshold value, and boolean for predicted debris flow (based on the two prior values) for stream segments.

@eculler @BCampforts

slundell123 and others added 30 commits August 9, 2021 20:40
Found that naming it something other than grid caused issues
when referencing it. Not sure if grid is protected in landlab
Adding dimensionless discharge component
Updating docs to include reference and equation.
Removing dt value from run_one_step function since
the value is not used currently.
True and false were switched
Updating tests to follow docs format
Adding component to list of docs for website
@kbarnhart
Copy link
Contributor

@slundell123 I've found that it is quite a bit easier for me to make some direct edits so I'll be pushing a few more commits with those changes for your consideration (will be pushed after I test them). If you don't like any of them, feel free to modify. The changes relate to the use of optional fields, the return_array constructor, and when the thresholds are calculated (this should also be in run one step, as slopes are updated in run one step).

@kbarnhart
Copy link
Contributor

@slundell123 I've finished going through this again. Nice work!

Other than the name & documentation of discharge per unit width, I think this is ready to go. @mcflugen may have other comments.

Let me know if you have any questions/comments about the changes I've pushed. Please consider them as suggestions. If you don't like something I did, please revise it away.

@slundell123
Copy link
Contributor Author

@kbarnhart your changes look good to me. I just updated the variable surface_water__discharge to surface_water__unit_discharge as discussed above. I'm not too sure how to change the field-related .rst files that you mentioned.

@slundell123
Copy link
Contributor Author

@mcflugen @kbarnhart Is there anything else I need to do on this so that it can be merged?

@kbarnhart
Copy link
Contributor

Will take a look in the next few days.

@kbarnhart
Copy link
Contributor

@slundell123 I don't think your description of the unit discharge was correct (or the units used). I've updated this. I'm fine with it being merged.

@mcflugen can you handle any final issues and merge this?

@mcflugen
Copy link
Member

@mcflugen can you handle any final issues and merge this?

Will do. Thanks for your help with this @kbarnhart!

mcflugen
mcflugen previously approved these changes Mar 19, 2022
Copy link
Member

@mcflugen mcflugen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mcflugen mcflugen merged commit 7fbd12a into landlab:master Apr 14, 2022
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.

4 participants