seabed stress - remove if statements #673
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -627,7 +627,6 @@ end subroutine dyn_prep2 | |
| subroutine stepu (nx_block, ny_block, & | ||
| icellu, Cw, & | ||
| indxui, indxuj, & | ||
| aiu, str, & | ||
| uocn, vocn, & | ||
| waterx, watery, & | ||
|
|
@@ -642,8 +641,7 @@ subroutine stepu (nx_block, ny_block, & | |
|
|
||
| integer (kind=int_kind), intent(in) :: & | ||
| nx_block, ny_block, & ! block dimensions | ||
| icellu ! total count when iceumask is true | ||
|
|
||
| integer (kind=int_kind), dimension (nx_block*ny_block), intent(in) :: & | ||
| indxui , & ! compressed index in i-direction | ||
|
|
@@ -1388,7 +1386,7 @@ subroutine viscous_coeffs_and_rep_pressure (strength, tinyarea, & | |
| real (kind=dbl_kind), intent(in):: & | ||
| Deltane, Deltanw, Deltasw, Deltase ! Delta at each corner | ||
|
|
||
| real(kind=dbl_kind) , intent(in):: capping | ||
|
|
||
| real (kind=dbl_kind), intent(out):: & | ||
| zetax2ne, zetax2nw, zetax2sw, zetax2se, & ! zetax2 at each corner | ||
|
|
@@ -1401,42 +1399,33 @@ subroutine viscous_coeffs_and_rep_pressure (strength, tinyarea, & | |
|
|
||
| ! NOTE: for comp. efficiency 2 x zeta and 2 x eta are used in the code | ||
|
|
||
|
There was a problem hiding this comment. I would prefer we keep this commented There was a problem hiding this comment. I reinserted it |
||
| tmpcalcne = capping *(strength/max(Deltane, tinyarea))+ & | ||
| (c1-capping)* strength/ (Deltane+ tinyarea) | ||
| tmpcalcnw = capping *(strength/max(Deltanw, tinyarea))+ & | ||
| (c1-capping)* strength/ (Deltanw+ tinyarea) | ||
| tmpcalcsw = capping *(strength/max(Deltasw, tinyarea))+ & | ||
| (c1-capping)* strength/ (Deltasw+ tinyarea) | ||
| tmpcalcse = capping *(strength/max(Deltase, tinyarea))+ & | ||
| (c1-capping)* strength/ (Deltase+ tinyarea) | ||
|
|
||
| zetax2ne = (c1+Ktens)*tmpcalcne ! northeast | ||
| rep_prsne = (c1-Ktens)*tmpcalcne*Deltane | ||
| etax2ne = epp2i*zetax2ne | ||
|
|
||
| zetax2nw = (c1+Ktens)*tmpcalcnw ! northwest | ||
| rep_prsnw = (c1-Ktens)*tmpcalcnw*Deltanw | ||
| etax2nw = epp2i*zetax2nw | ||
|
|
||
| zetax2sw = (c1+Ktens)*tmpcalcsw ! southwest | ||
| rep_prssw = (c1-Ktens)*tmpcalcsw*Deltasw | ||
| etax2sw = epp2i*zetax2sw | ||
|
|
||
| zetax2se = (c1+Ktens)*tmpcalcse ! southeast | ||
| rep_prsse = (c1-Ktens)*tmpcalcse*Deltase | ||
| etax2se = epp2i*zetax2se | ||
|
|
||
| end subroutine viscous_coeffs_and_rep_pressure | ||
|
|
||
| !======================================================================= | ||
|
|
||
| ! Load velocity components into array for boundary updates | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this change is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If ndte=1 then some compilers will create warnings and maybe even stop due to a do loop that runs from 1 to 0. The same should be included in the 1d .Now it aborts if ndte < 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't be true that the code will stop? Compiler warnings maybe, but setting loop values so a loop won't be executed is done all the time and should be allowed in general. I'm not opposed to the if check, it's not going to affect performance, but I think it's cleaner not being there. Where is the check for ndte<1. It's not in this part of the code, so the if check is needed even less.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a strong motivator is to get rid of if checks in loops. I assume this is for performance. My question continues to be whether we have measured a performance benefit with these changes. If not, then I prefer that we not replicate the calls for the last subcycle step in order to add the deformations call outside an if statement. My suspicion is that the if statement in the subcycling loop is not going to affect performance. Getting rid of if statements inside an ij loop, ok maybe, but not at this high a level in the computations. In my opinion, it's easier to maintain if the last iteration is not separated/duplicated. Maybe there are other reasons for separating that last subcycle. But if not, then I still prefer we not do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree in principle with getting rid of if checks in loops. They can make a big difference in performance, but it depends on the loop size and things like how long the pipeline is for alternative commands, which probably varies a good bit across hardware/compiler configurations. Maybe it's gotten better with newer generations. The other considerations here are not having duplicative code (which I also agree with) and making close-but-not-quite-duplicated code for various dynamics options as similar as possible. Since the dynamics is typically the most expensive part of the sea ice simulation, and centers seem to be moving toward higher numbers of subcycles for better convergence, then optimizing these loops seems wise. It's hard to judge, though, if the existing timing tests are inconclusive. Question: in the original code, the deformations routine is called at the end, rather than during the loop. Is the argument for moving it up into the loop (or interrupting the loop) that the computed deformations need to be consistent with the velocities just prior to the last iteration? Or is it not better that the deformations be computed from the final velocities? (Maybe I'm not understanding something here?)