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

Add IO-free driver code #3

Merged
merged 39 commits into from
Jun 20, 2022
Merged

Conversation

samhatfield
Copy link
Collaborator

No description provided.

@FussyDuck
Copy link

FussyDuck commented Jun 9, 2022

CLA assistant check
All committers have signed the CLA.

@samhatfield
Copy link
Collaborator Author

This is not ready for merging yet. I just opened the PR for others to take a look.

@samhatfield
Copy link
Collaborator Author

Example benchmark from

OMP_NUM_THREADS=<number of threads> mpirun -n <number of ranks> ./bin/driver-spectrans-sp 79 10 1

:

Number of ranks | OMP_NUM_THREADS | Total wallclock time | Relative
              1 |               1 |               10.537 | 1.00
              1 |               2 |                6.226 | 0.59
              1 |               4 |                4.161 | 0.39
              1 |               8 |                4.607 | 0.44
              2 |               4 |                3.598 | 0.34

@samhatfield
Copy link
Collaborator Author

samhatfield commented Jun 9, 2022

New help message:

$ ./bin/driver-spectrans-sp -h

NAME    driver-spectrans-sp
DESCRIPTION
        This program tests ecTrans by transforming fields back and forthbetween spectral 
        space and grid-point space (single-precision version)

USAGE
        driver-spectrans-sp [options]
OPTIONS
        -h      Print this message
        -n iterations
                Run for this many inverse/direct transform iterations
        -t truncation
                Run with this triangular spectral truncation


@wdeconinck
Copy link
Collaborator

wdeconinck commented Jun 9, 2022

Very nice Sam. I have added a little more options and added unit test for it:

NAME    ectrans-benchmark1-dp

DESCRIPTION
        This program tests ecTrans by transforming fields back and forth between spectral 
        space and grid-point space (double-precision version)

USAGE
        ectrans-benchmark1-dp [options]

OPTIONS
    -h                  Print this message
    -v                  Run with verbose output
    -t, --truncation T  Run with this triangular spectral truncation (default = 79)
    -g, --grid GRID     Run with this grid. Possible values: O<N>, F<N>
                        If not specified, O<N> is used with N=truncation+1 (cubic relation)
    -n, --niter NITER   Run for this many inverse/direct transform iterations (default = 10)
    -f, --nfld NFLD     Number of scalar fields (default = 1)

A few things to note.

  1. It seems that nprtrv and nprtrw are undefined, see some failing tests.
  2. The --nfld option is for now not used
    We could already merge I believe if these items are sorted

Further down the line we may need to extend to also use the vorticity-divergence to wind option simultaneously or separately. I think @ioanhadade has a full featured driver somewhere that we can look at?

@samhatfield
Copy link
Collaborator Author

Good spot on the uninitialised variables - fixed.

There is currently a very dumb way of visualising fields that simply dumps all grid points values serially to unformatted binary files. I have a Python script which can reassemble these files but it only works for a single rank. This was super useful when testing things like half precision as you can instantly spot errors that would be hard to see just from looking at norms. Would you be okay with leaving this feature in and making it off by default, and activated with, say, -d --dump-values?

@wdeconinck
Copy link
Collaborator

Good spot on the uninitialised variables - fixed.

That's what unit tests are for ;)

There is currently a very dumb way of visualising fields that simply dumps all grid points values serially to unformatted binary files. I have a Python script which can reassemble these files but it only works for a single rank. This was super useful when testing things like half precision as you can instantly spot errors that would be hard to see just from looking at norms. Would you be okay with leaving this feature in and making it off by default, and activated with, say, -d --dump-values?

Sure no problem with that, but perhaps not with the '-d', just with '--dump-values'. We may want to keep the '-d' in our pocket.

@marsdeno
Copy link
Collaborator

Very nice work guys, will be very useful!

@wdeconinck
Copy link
Collaborator

I further adapted the benchmark / test so that it works without MPI as well, e.g. with mpi_serial backend of fiat.
And I implemented more options, making e.g. vordiv2uv, scalar derivatives and uv derivatives an "opt in".
That will be useful for unit tests to narrow down what works and doesn't, especially when porting to gpu.
Many more tests are added that test the benchmark in various configurations in low resolution (T47-O48).

@wdeconinck
Copy link
Collaborator

I'm happy to merge this in now as a first checkpoint.

@wdeconinck wdeconinck requested review from wdeconinck, marsdeno and ioanhadade and removed request for wdeconinck June 10, 2022 17:05
Copy link
Collaborator

@wdeconinck wdeconinck left a comment

Choose a reason for hiding this comment

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

A bit more testing is probably advised, to make sure all options do as they are promising, but to me it is already a great start.

Copy link
Collaborator

@marsdeno marsdeno left a comment

Choose a reason for hiding this comment

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

Down the line I hope we can increase coverage of the tests, and differentiate correctness checking from performance driver, but this is excellent way to start in that direction.

@wdeconinck wdeconinck merged commit d409277 into ecmwf-ifs:main Jun 20, 2022
@samhatfield samhatfield deleted the sh/io_free_driver branch July 19, 2022 16:19
ddegrauwe pushed a commit to ddegrauwe/ectrans that referenced this pull request Apr 19, 2023
…nges

Optimisations from AMD GPU hackathon
dareg pushed a commit to dareg/ectrans_nvidia that referenced this pull request Apr 15, 2024
Always build with MPI, regardless of GPU-aware option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants