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

Update source and online documentation of silt fraction #54

Merged
merged 4 commits into from
Sep 22, 2023

Conversation

anne-glerum
Copy link
Contributor

This PR updates the online documentation and the comments in Fastscape_api.f90 to

  • use the term 'silt fraction' instead of 'sand-shale ratio'.
  • replace 'shale' by 'silt'.
  • switch the online description of the order of sand and silt variables in the FastScape_Set_Marine_Parameters function.

These changes unify the terminology with that used in Marine.f90 and the paper by Yuan et al. (2019) in EPSL. The order of the FastScape_Set_Marine_Parameters arguments in the manual is now also in agreement with the order specified in lines 43-52 of FastScape_api.f90.

Only in Strati.f90 is Sand/ShaleRatio still used as output parameter name (line 62); I didn't change it because it actually changes the output, not just the documentation. Let me know if I should also change that parameter name.

FYI @Djneu

@benbovy
Copy link
Member

benbovy commented May 26, 2023

Many thanks for the feedback and for taking care of this @anne-glerum.

Only in Strati.f90 is Sand/ShaleRatio still used as output parameter name (line 62); I didn't change it because it actually changes the output, not just the documentation. Let me know if I should also change that parameter name.

I think the improved consistency would be worth the small breaking change here.

Note for myself: I'll need to update the marine component in https://github.com/fastscape-lem/fastscape accordingly.

@anne-glerum
Copy link
Contributor Author

Many thanks for the feedback and for taking care of this @anne-glerum.

You're welcome!

I think the improved consistency would be worth the small breaking change here.
Okay, I've updated the output name as well.

It seems my git or vi has also unintentionally changed the EOL of FastScape_api.f90. I'm trying to undo that, unsuccessfully so far.

Do I need to worry about the failing testers? I don't think that changing the comments could cause these errors.

@benbovy
Copy link
Member

benbovy commented May 26, 2023

Do I need to worry about the failing testers? I don't think that changing the comments could cause these errors.

You can safely ignore them, they are unrelated to the changes here and they have been fixed in #56.

@benbovy benbovy merged commit 7f144cd into fastscape-lem:master Sep 22, 2023
@benbovy
Copy link
Member

benbovy commented Sep 22, 2023

Thanks again @anne-glerum!

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