Negatises anfix #46
Merged
Negatises anfix #46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Matt
I think this is what we were talking about today - nfix not conserving mass with internal nutrients simulated.
It is down to signs of various quantities. When the flux of N is computed on the line below, nuptake(phy_i,:) is summed to get total uptake, and the sum is negatised. This is fine for amm and no3 - their uptakes are negative quantities, so summing them and negatising the sum provides a positive uptake quantity.
Nitrogen fixing uptake at nuptake(phy_i,4) (anfix) however is computed as a positive uptake quantity and so adding it to the negative amm and no3 uptakes means that the terms partially cancel, and are not added up correctly: a positive anfix uptake is added to the two negative amm and no3 uptakes, and then the sum negatised. I included the debug info on the right hand side of the screen shot below so you can see the signs of the elements of the nuptake array.
I have suggested the below fix in aed_bio_utils, that is, reporting anfix uptake as a negative quantity back to aed_phytoplankton. This way, all the summed uptakes are the same sign (negative) before the sum is negatised. This suggestion might have flow on effects elsewhere in the code though, so I am not sure.
When I make this change in my local repo I get mass to conserve to 0.2% over a month. Without the fix the divergence was greater than 20%.
MB
Quick update: the new diagnostic below will also be affected by the sign of anfix. The sum on nuptake will be potentially cancelling rather than additive.