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

414 merge am3 miscellaneous science modifications #430

Conversation

JhanSrbinovsky
Copy link
Collaborator

@JhanSrbinovsky JhanSrbinovsky commented Oct 21, 2024

CABLE

Thank you for submitting a pull request to the CABLE Project.

Description

Miscellaneous developments in science/ dir. from AM3. These are included separately to more major developments in canopy and soilsnow directories.

Fixes #402

Type of change

Including AM3 development.

Checklist

  • [X ] The new content is accessible and located in the appropriate section.
  • [ X] I have checked that links are valid and point to the intended content.
  • [ X] I have checked my code/text and corrected any misspellings

📚 Documentation preview 📚: https://cable--430.org.readthedocs.build/en/430/

@JhanSrbinovsky JhanSrbinovsky linked an issue Oct 21, 2024 that may be closed by this pull request
@JhanSrbinovsky
Copy link
Collaborator Author

Not ready to submit for merge as dependent on preceding merge of #402, params

see benchcab analysis

@JhanSrbinovsky JhanSrbinovsky added blocked Issue that requires other work to be done first priority:high High priority issues that should be included in the next release. labels Oct 21, 2024
@JhanSrbinovsky JhanSrbinovsky self-assigned this Oct 21, 2024
@JhanSrbinovsky JhanSrbinovsky changed the base branch from main to 402-merge-from-am3-params October 21, 2024 22:26
@JhanSrbinovsky
Copy link
Collaborator Author

Note: comparison here is to branch #402

Copy link
Collaborator

@har917 har917 left a comment

Choose a reason for hiding this comment

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

Looks okay but I think we've gone backwards with the changes to the spitter call - Needs checking.

@JhanSrbinovsky
Copy link
Collaborator Author

One last thing - did you look at the benchcab analysis? I couldn't really make any sense of it. Some slightly better, some slightly worse it seemed.

@JhanSrbinovsky JhanSrbinovsky merged commit 1717d92 into 402-merge-from-am3-params Oct 28, 2024
7 checks passed
@JhanSrbinovsky JhanSrbinovsky deleted the 414-merge-am3-miscellaneous-science-modifications branch October 28, 2024 23:42
@JhanSrbinovsky JhanSrbinovsky restored the 414-merge-am3-miscellaneous-science-modifications branch October 29, 2024 01:56
@JhanSrbinovsky
Copy link
Collaborator Author

DOH!! Because of the lag in submitting these to main, and they are all sequential , and somewhat dependent on the previous tranche of developments, I branched this (or at least compared it to the pre-ceding, related issue #402 ). Unfortunately this hadn't gone through, I hadn't remembered weeks later that I had "compared" to the 402 branch in this PR and so when merging was approved/executed - it went back to the params branch.

In the interim I have addressed the comments about cable_surface_types in #436. This PR will/should go through fairly quickly. Regardless, I need to, it will be cleaner if I rebase from main post #436 going in AND then re-submit this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Issue that requires other work to be done first priority:high High priority issues that should be included in the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge AM3 (miscellaneous) science modifications
3 participants