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

update mpif.h to use mpi #627

Merged
merged 1 commit into from
Sep 24, 2024
Merged

update mpif.h to use mpi #627

merged 1 commit into from
Sep 24, 2024

Conversation

sebastianbeyer
Copy link
Collaborator

Locally compiles, let's see what the tests say. Also, do we test compiling with e.g. ifs_interface enabled? Otherwise we might be missing something ;)
I will try use mpi_f08 as well if this works and also try to build on levante and lumi...

@sebastianbeyer
Copy link
Collaborator Author

I could build and run on levante and I also noticed that mpi_topology_module.F90 already contains use mpi in one place ;)
use mpi_f08 takes more effort (for some reason the general mpi routines can not be found, but if I call everything with _f08 postfix it does find it...) so I would suggest to merge this now and look at f08 later again.

@sebastianbeyer sebastianbeyer marked this pull request as ready for review September 22, 2024 11:52
@sebastianbeyer sebastianbeyer added the enhancement New feature or request label Sep 22, 2024
@sebastianbeyer sebastianbeyer self-assigned this Sep 22, 2024
@JanStreffing
Copy link
Collaborator

Standalone looks good. I assume you tested this with IFS interface by now? Our test suit checks if FESOM builds as a library, but obviously it can't check if it runs as a library in IFS.

@sebastianbeyer
Copy link
Collaborator Author

I am testing with IFS now, but it takes a little, because we are behind main quite a bit and now I need to add all of the namelist groups that did not exist before to the config on the HPCs...

@sebastianbeyer
Copy link
Collaborator Author

okay, now I tested on levante with IFS-FESOM and it still works. So from my point of view this can be merged

@JanStreffing JanStreffing added this to the FESOM 2.6 milestone Sep 24, 2024
@JanStreffing JanStreffing merged commit a560e74 into main Sep 24, 2024
4 checks passed
@JanStreffing JanStreffing deleted the update_mpi branch September 24, 2024 09:47
@ackerlar
Copy link
Collaborator

ackerlar commented Oct 8, 2024

I got the following error during compilation:

/home/a/a270124/model_codes/awiesm-2.1-paleodyn-2.5/fesom-2.5/src/cpl_driver.F90(354): error #6285: There is no matching specific subroutine for this generic subroutine call.   [MPI_COMM_RANK]
    CALL MPI_Comm_Rank ( MPI_COMM_WORLD, commRank, ierror )
---------^
compilation aborted for /home/a/a270124/model_codes/awiesm-2.1-paleodyn-2.5/fesom-2.5/src/cpl_driver.F90 (code 1)

#634 solves the issue for me

@sebastianbeyer
Copy link
Collaborator Author

You need to fix the call, probably you need to use the correct type for MPI_COMM_WORLD

@sebastianbeyer
Copy link
Collaborator Author

sebastianbeyer commented Oct 8, 2024

ah, no it's not that...
looks like this file is not always compiled. Seems like it does call MPI subroutines, but there is no use mpi nor include mpif.h anywhere. strange.

@sebastianbeyer
Copy link
Collaborator Author

Okay, it looks like the file cpl_driver.F90 does not contain either use mpi nor include mpif.h. This was never correct, but before it did work, because when that file included another module which did have include mpif.h it would also get a copy of the subroutines. So the solution is to rather add use mpi to the respective modules in cpl_driver.F90.

@sebastianbeyer
Copy link
Collaborator Author

and also update the CI, so that this file is also built during in the CI builds.

@sebastianbeyer sebastianbeyer mentioned this pull request Oct 8, 2024
@ackerlar
Copy link
Collaborator

ackerlar commented Oct 9, 2024

Unfortunately, I still get the same error during compilation

@sebastianbeyer
Copy link
Collaborator Author

sebastianbeyer commented Oct 9, 2024 via email

@JanStreffing
Copy link
Collaborator

You can install awi-esm2 with esm_tools. that should give you the error. @ackerlar, which version does Sebastian have to try and install?

@ackerlar
Copy link
Collaborator

ackerlar commented Oct 9, 2024

The version is "awiesm-2.1-paleodyn-2.5"

@sebastianbeyer
Copy link
Collaborator Author

But that requires access to https://git.geomar.de/foci/src/oasis3mct.git if I understand correctly. And I don't have that...

@JanStreffing
Copy link
Collaborator

That doesn't sound right, awi-esm2 should not use geomar oasis... should be from https://gitlab.dkrz.de/modular_esm/oasis3-mct i think

@ackerlar
Copy link
Collaborator

ackerlar commented Oct 9, 2024

Apparently, oasis and echam come from the AWI gitlab:

https://gitlab.awi.de/paleodyn/Models/oasis3-mct.git
https://gitlab.awi.de/paleodyn/Models/echam6.git

@mandresm or @pgierz could give you access

@sebastianbeyer
Copy link
Collaborator Author

ah, I did not see the other possible repositories.

Okay, but this is too tedious, sorry

@JanStreffing
Copy link
Collaborator

I downloaded the source codes, you can copy from here /work/ab0246/a270092/model_codes/awiesm-2.1-paleodyn-2.5 and do esm_master comp-awiesm-2.1-paleodyn-2.5 in the copy you copy it to.

@mandresm
Copy link
Collaborator

You should have access now @sebastianbeyer

@sebastianbeyer
Copy link
Collaborator Author

Okay, the line that you show: CALL MPI_Comm_Rank ( MPI_COMM_WORLD, commRank, ierror )
has commRank and that is a logical! but MPI_COMM_RANK expects an integer here! that is the reason to do use mpi instead of include mpif.h! Type safety!
What is the purpose of commRank? what should be its type??

@ackerlar
Copy link
Collaborator

It seems to me that commRank is only used for few write statements. I changed its type and adapted the if statements (I don't know who included this part but I guess it should read ioerrer .eq. oasis_ok) and now the models compiles. I a doing a quick test run and push the changes. Thanks @sebastianbeyer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants