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

add riverine C-isotope input #277

Merged
merged 2 commits into from
Oct 3, 2023
Merged

add riverine C-isotope input #277

merged 2 commits into from
Oct 3, 2023

Conversation

monsieuralok
Copy link
Collaborator

No description provided.

@jmaerz
Copy link
Collaborator

jmaerz commented Oct 3, 2023

Hi @monsieuralok , many thanks for bringing in these changes! - just one thing: all the sedbypass flags in powach and sedshi are not really needed, since powach and sedshi are only called, when sedbypass=.false. (see hamocc4bgm.F90).

@monsieuralok
Copy link
Collaborator Author

Hi @monsieuralok , many thanks for bringing in these changes! - just one thing: all the sedbypass flags in powach and sedshi are not really needed, since powach and sedshi are only called, when sedbypass=.false. (see hamocc4bgm.F90).

@jmaerz Jerry mentioned that it could be we can have both TRUE also that what we checked today and yesterday. But, you should also check it.

@monsieuralok
Copy link
Collaborator Author

Hi @monsieuralok , many thanks for bringing in these changes! - just one thing: all the sedbypass flags in powach and sedshi are not really needed, since powach and sedshi are only called, when sedbypass=.false. (see hamocc4bgm.F90).

@jmaerz Jerry mentioned that it could be we can have both TRUE also that what we checked today and yesterday. But, you should also check it.

sorry. I understand what you mentioned. I will update.

Copy link
Contributor

@JorgSchwinger JorgSchwinger left a comment

Choose a reason for hiding this comment

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

I agree with Jöran that the changes in powach and sedchi are not necessary - these routines are only called when sediment-bypass is disabled. Otherwise I can approve this - thanks for bringing this in!

@monsieuralok
Copy link
Collaborator Author

I agree with Jöran that the changes in powach and sedchi are not necessary - these routines are only called when sediment-bypass is disabled. Otherwise I can approve this - thanks for bringing this in!

@JorgSchwinger @jmaerz It would be then good to pick file mo_apply_rivin.F90 only. Or should I send it again?

@jmaerz
Copy link
Collaborator

jmaerz commented Oct 3, 2023

Hi @monsieuralok , I would suggest to revert the changes for files sedshi and powach and then push again to the same branch. Either just copy the original files or there seem to be the git way (I just found it on stackoverflow: https://stackoverflow.com/questions/2620775/revert-changes-to-a-file-in-a-commit):

git show some_commit_sha1 -- some_file.c | git apply -R

(and add + commit + push to the same github branch) Hope that helps. Thanks again for bringing the stuff in!

@JorgSchwinger
Copy link
Contributor

Yes would be easiest if you could update this PR, then we can just merge it from here.

@monsieuralok
Copy link
Collaborator Author

Hi @monsieuralok , I would suggest to revert the changes for files sedshi and powach and then push again to the same branch. Either just copy the original files or there seem to be the git way (I just found it on stackoverflow: https://stackoverflow.com/questions/2620775/revert-changes-to-a-file-in-a-commit):

git show some_commit_sha1 -- some_file.c | git apply -R

(and add + commit + push to the same github branch) Hope that helps. Thanks again for bringing the stuff in!

@JorgSchwinger @jmaerz I have updated my repo

@JorgSchwinger JorgSchwinger changed the title changes for isotopes read/write add riverine C-isotope input Oct 3, 2023
@JorgSchwinger JorgSchwinger merged commit 4556746 into NorESMhub:master Oct 3, 2023
10 checks passed
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.

3 participants