Add ability to call ecTrans with two "call modes" in benchmark #122
Conversation
This file contains hidden or 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
samhatfield
changed the title
samhatfield
force-pushed
the
samhatfield/refactor_benchmark_driver
branch
from
74ea10a to
b4fa742
Compare
|
Finally ready for review, when you're ready @marsdeno @wdeconinck. |
samhatfield
commented
Sep 11, 2024
samhatfield
force-pushed
the
samhatfield/refactor_benchmark_driver
branch
from
7433c65 to
2b57504
Compare
|
Hello, any update regarding this ? it is still blocking HPCW release. |
|
Hi @antoine-morvan - I'm afraid we just have to be patient with this. This PR is not considered important on our side, and the colleagues responsible for the review are not connected to the HPCW project. Hence we cannot ask them to prioritise this work. I think there is a way to make the first HPCW release even before this PR is merged -> I'll make a comment on the DKRZ Gitlab issue. |
samhatfield
force-pushed
the
samhatfield/refactor_benchmark_driver
branch
from
8f4e8a7 to
24702d0
Compare
samhatfield
force-pushed
the
samhatfield/refactor_benchmark_driver
branch
from
24702d0 to
a309b68
Compare
samhatfield
force-pushed
the
samhatfield/refactor_benchmark_driver
branch
2 times, most recently
from
e47a267 to
84ee0a0
Compare
|
Perhaps a good time to resolve conflicts and merge now? |
samhatfield
force-pushed
the
samhatfield/refactor_benchmark_driver
branch
from
8dc2b1c to
cafb5bd
Compare
INV_TRANS and DIR_TRANS support two ways of passing arrays: 1) PSPVOR, PSPDIV, PSPSCALAR <=> PGP 2) PSPVOR, PSPDIV <=> PGPUV; PSPSC3A <=> PGP3A; PSPSC2 <=> PGP2 Previously we only supported the second one. With this commit you can now also call the first style. In the IFS we use both cases in different places so it's important to have both cases covered. We can now extend the test suite so more of the code has coverage. In this commit I also tidied up a few things: - All spectral and grid point work arrays are separately allocated, rather than relying on big work arrays to store everything and pointers to access slices. - I deleted a couple unused variables.
This needs to be investigated.
samhatfield
force-pushed
the
samhatfield/refactor_benchmark_driver
branch
from
cafb5bd to
91903d8
Compare
wdeconinck
approved these changes
Jul 31, 2025
| if( NOT "${benchmark}" MATCHES "-gpu-" ) | ||
| ecbuild_add_test( TARGET ${base_title}_nfld0 | ||
| COMMAND ${benchmark} | ||
| ARGS --truncation ${t} --grid ${grid} --nfld 0 --check 200 ${base_args} --callmode ${callmode} |
There was a problem hiding this comment.
Better to keep base_args at the end?
| COMMAND ${benchmark} | ||
| ARGS --truncation ${t} --grid ${grid} --nfld 0 --check 100 ${base_args} | ||
| ARGS --truncation ${t} --grid ${grid} --nfld 10 --check 200 ${base_args} --callmode ${callmode} |
There was a problem hiding this comment.
why the change in check argument?
| @@ -1143,6 +1125,7 @@ & subroutine on memory usage, thread-binding etc." | |||
| & tolerance for correctness checking" | |||
| write(nout, "(a)") " --no-pinning Disable memory-pinning (a.k.a. page-locked memory) & | |||
| & to allocate fields for GPU version" | |||
| write(nout, "(a)") " --callmode The call mode for INV_TRANS and DIR_TRANS (1 or 2)" | |||
There was a problem hiding this comment.
Could we have some more documentation on this mode? Also as part of the help.
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.
INV_TRANS and DIR_TRANS support two ways of passing arrays:
Previously we only supported the second one. With this commit you can now also call the first style. In the IFS we use both cases in different places so it's important to have both cases covered.
We can now extend the test suite so more of the code has coverage.
I am still trying to figure out how scalar fields are packed, hence this branch gives erroneous norms for scalar fields. I will continue working on this.