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

[telemac] added telemac support #176

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[telemac] added telemac support #176

wants to merge 1 commit into from

Conversation

tomsail
Copy link
Collaborator

@tomsail tomsail commented Mar 19, 2024

No description provided.

@tomsail
Copy link
Collaborator Author

tomsail commented Mar 19, 2024

let me battle with CI before starting reviewing it. There is no rush anyway

@tomsail tomsail marked this pull request as draft March 19, 2024 19:56
@tomsail
Copy link
Collaborator Author

tomsail commented Mar 20, 2024

I might have broken SCHISM tests with mpi4py.
I think it might be relevant to create a fourth lock:

SCHISM_LOCK: locks/conda-${{ matrix.os }}-schism_${{ matrix.mpi }}.lock
DELFT3D_LOCK: locks/conda-${{ matrix.os }}-delft3d_${{ matrix.mpi }}.lock
PYPOSEIDON_LOCK: locks/conda-${{ matrix.os }}-binary-p${{ matrix.python }}.lock
PYPOS_TEL_LOCK: locks/conda-${{ matrix.os }}-binary-p${{ matrix.python }}_tel.lock

So I don't mess up with the other CIs

@tomsail tomsail marked this pull request as ready for review March 20, 2024 11:07
@tomsail
Copy link
Collaborator Author

tomsail commented Mar 20, 2024

I think that:

  • removing mpi4py in pyproject.toml
  • and adding it with pip install mpi4py in the CI
    is very bad .. but I couldn't find another way to make CI work

@pmav99
Copy link
Collaborator

pmav99 commented Mar 20, 2024

I updated all the lock files. The CI seems to be happy. The telemac are marked as XPASS though. @tomsail did you expect them to fail for some reason?

@tomsail
Copy link
Collaborator Author

tomsail commented Mar 21, 2024

I updated all the lock files. The CI seems to be happy. The telemac are marked as XPASS though. @tomsail did you expect them to fail for some reason?

No sorry this is an error. I had put it at the beginning and forgot to take it off.

Since the 4 tests (3 TELEMAC2D and 1 TOMAWAC) pass without problems I can remove it now.

@pmav99
Copy link
Collaborator

pmav99 commented Apr 8, 2024

@tomsail if you have any updates on your local branch you can push them. We will need to rebase and we will need to regenerate lock files etc.

@tomsail
Copy link
Collaborator Author

tomsail commented Apr 9, 2024

@pmav99 let me know if you want me to rebase or if you want to check it too

@tomsail
Copy link
Collaborator Author

tomsail commented Apr 13, 2024

I still need to regroup the telemac options in parameters, and standardize them with the ones used for SCHISM.

If you compare the specific parameters for SCHISM and TELEMAC in the same dictionary:

model = {
   ...
    "parameters": {
        "dt": 400,
        "rnday": 30,
        "hotout": 0,
        "ihot": 0,
        "nspool": 9,
        "ihfskip": 36,
        "hotout_write": 108,
    },  # FOR SCHISM
    "tstep": 400, # for telemac
    "meteo_input360": True,  # if meteo files longitudes go from from 0 to 360
}

For example here tstep should be dt and be placed in parameters.

I'll correct this in the next days

@tomsail tomsail force-pushed the telemac branch 2 times, most recently from c970f85 to 8f14cb3 Compare July 15, 2024 11:57
@tomsail
Copy link
Collaborator Author

tomsail commented Jul 15, 2024

fail with python 3.12 is normal.
There has been some update regarding this issue actually: https://gitlab.pam-retd.fr/otm/telemac-mascaret/-/issues/946

@tomsail tomsail force-pushed the telemac branch 2 times, most recently from 8c9dbaf to a8ead8b Compare September 14, 2024 13:43
@tomsail
Copy link
Collaborator Author

tomsail commented Sep 14, 2024

CI fail because of numpy=2.x.x which is not implemented in TELEMAC yet
https://github.com/ec-jrc/pyPoseidon/actions/runs/10862902401/job/30146448815

@tomsail
Copy link
Collaborator Author

tomsail commented Dec 10, 2024

We can regenerate lock files and use opentelemac from the conda-forge channel instead of mine.
test python 3.12 fail because we don't have the corresponding lock file.

@tomsail
Copy link
Collaborator Author

tomsail commented Dec 12, 2024

ready for final review

Copy link
Collaborator

@brey brey left a comment

Choose a reason for hiding this comment

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

I am ok with the changes to core files. I will revise if need be in my PR.

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.

3 participants