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

cleaned up plankton foodweb code #67

Merged
merged 2 commits into from
Jun 11, 2024
Merged

Conversation

charliestock
Copy link
Contributor

This pull request cleans up the code associated with zooplankton and higher predator grazing. This involved:

  1. Removing legacy code from COBALTv2 that had just been commented out
  2. Removing code associated with an experimental diapause approximation that was not being used.
  3. Removing an experimental "self grazing" setting that was not being used.
  4. Improved commenting throughout.

The removal of the experimental diapause formulation means that the diagnostic tracer "irr_mem_dp" is no longer needed. Several diagnostics were also removed in the clean up, but none of these seems to appear in the diagnostic table.

I have tested the code and it runs. This should not change answers relative to the last version, and it ultimately removed about 100 lines of code.

@charliestock charliestock added documentation Improvements or additions to documentation CodeCleanUp labels Jun 10, 2024
Copy link
Collaborator

@yichengt900 yichengt900 Jun 10, 2024

Choose a reason for hiding this comment

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

@charliestock , Thanks for opening this PR. Overall the changes are good. Since you are cleaning up the plankton section, I'd like to ask your opinion on the vertical movement section for both phytoplankton and zooplankton. Do we want to keep this capability, or would we prefer to revert it back to COBALTv2? If we decide to keep it, I suggest we fix the parameter name issue for swim_max in this PR too.

Also I can confirm this PR does not change answers in NWA12-RT case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi YC - I think we can remove the zooplankton movement for now, but we need to keep the phytoplankton sinking. @jessluo will eventually have a more comprehensive & correct movement formulation to add.

I can do a follow up pull request for the zooplankton swimming tomorrow, but perhaps we can pull in the foodweb changes today. This was mainly what I was hoping to cover in tomorrow's meeting, so a clean code would be helpful.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@charliestock Thanks. It sounds like a plan to me. I don't want to be annoyingly picky, but the swim_max issue generates a bunch of annoying warning messages during model runtime. I'll be happy to see it getting fixed in the next PR. Other than that, this PR looks good, except for one super tiny comment I left above. I will approve this PR once it is resolved.

Copy link
Collaborator

@jessluo jessluo left a comment

Choose a reason for hiding this comment

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

looks good to me!

@charliestock
Copy link
Contributor Author

Great, thanks @yichengt900 and @jessluo - why don't we blend this in today and the next pull requests can build from this.

best,
Charlie

Added missing closing quotation mark
@yichengt900 yichengt900 merged commit 3c6288c into dev/cefi Jun 11, 2024
1 check passed
@yichengt900 yichengt900 deleted the foodweb_cleanup/clean_code branch August 1, 2024 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CodeCleanUp documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants