58 revise layer distribution #59
Merged
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
…fferences (max 1cm), also made the delta_time check towards time steps and phase-wise phase lengths a bit more strict to prevent having corrected phase lenghts of 0, wich doesn't allow for any correction in a new phase.
…sea volumes are zero in phases where that door is closed.
vrielin
approved these changes
Jan 21, 2025
There was a problem hiding this comment.
I don't think you made a typo in any of the new lines for to and from quantities.
But please add 1 comment line in sealock.h to explain. It took me, with the complete background of the development process, also quite some time to understand why this distinction is important.
vrielin
reviewed
Jan 21, 2025
|
I checked that there is 1 unit test for (changed) method distribute_discharge_over_layers. I don't think that this new test covers all if statements in the method, but it exists. If you think more unit testing is useful for this, please add it to the wish-list for later this year. |
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.
See issue decriptions