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

Support JRA55-do #45

Merged
merged 12 commits into from
Jul 19, 2023
Merged

Support JRA55-do #45

merged 12 commits into from
Jul 19, 2023

Conversation

ezhilsabareesh8
Copy link
Contributor

Updated datm_datamode_jra_mod.f90 to support JRA-55 do

Copy link
Contributor

@aekiss aekiss Jul 12, 2023

Choose a reason for hiding this comment

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

this file should be moved to a JRA55do1.4.0-RYF fork of the MOM6-CICE6 configuration https://github.com/COSIMA/MOM6-CICE6

Copy link
Contributor

Choose a reason for hiding this comment

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

This file also needs to be updated to use the RYF inputs from /g/data/ik11/inputs/JRA-55/RYF/v1-4 - see fields with "domain": "land" in https://github.com/COSIMA/1deg_jra55_ryf/blob/master/atmosphere/forcing.json

Copy link
Contributor

@aekiss aekiss Jul 12, 2023

Choose a reason for hiding this comment

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

this file should be moved to a JRA55do1.4.0-RYF fork of the MOM6-CICE6 configuration https://github.com/COSIMA/MOM6-CICE6

Copy link
Contributor

Choose a reason for hiding this comment

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

So there will need to be a corresponding PR in https://github.com/COSIMA/MOM6-CICE6

Copy link
Contributor

Choose a reason for hiding this comment

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

This file also needs to be updated to use the RYF inputs from /g/data/ik11/inputs/JRA-55/RYF/v1-4 - see https://github.com/COSIMA/1deg_jra55_ryf/blob/master/atmosphere/forcing.json

Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to pass this up to CDEPS, so to avoid clashing with the original it should be renamed as datm_datamode_jra55do_mod.F90 and the module and subroutine names changed to match. Also need to duplicate and modify this code block

@@ -30,8 +30,8 @@ list(APPEND cdeps_common_src_files
CDEPS/datm/datm_datamode_cplhist_mod.F90
CDEPS/datm/datm_datamode_era5_mod.F90
CDEPS/datm/datm_datamode_gefs_mod.F90
CDEPS/datm/datm_datamode_jra_mod.F90

#CDEPS/datm/datm_datamode_jra_mod.F90
Copy link
Contributor

Choose a reason for hiding this comment

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

don't comment out

CDEPS/datm/datm_datamode_jra_mod.F90

#CDEPS/datm/datm_datamode_jra_mod.F90
extra_sources/datm_datamode_jra_mod.F90
Copy link
Contributor

Choose a reason for hiding this comment

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

change to extra_sources/datm_datamode_jra55do_mod.F90

@aekiss
Copy link
Contributor

aekiss commented Jul 12, 2023

Thanks @ezhilsabareesh8. I've made a few suggestions - if you push these changes to this branch the pull request will be automatically updated

This was referenced Jul 12, 2023
…mod.F90

Module and subroutine names are changed to jra55do
Changed file path from extra_sources/datm_datamode_jra55_mod.F90 to extra_sources/datm_datamode_jra55do_mod.F90
@aekiss
Copy link
Contributor

aekiss commented Jul 12, 2023

Thanks for the update @ezhilsabareesh. There's an issue with cmake - see checks

@aekiss
Copy link
Contributor

aekiss commented Jul 12, 2023

ah, never mind - I see you're onto it already!

Module and subroutine names are changed to jra55do
Updated to use the RYF inputs from /g/data/ik11/inputs/JRA-55/RYF/v1-4
Updated to use the RYF inputs from /g/data/ik11/inputs/JRA-55/RYF/v1-4
@ezhilsabareesh8
Copy link
Contributor Author

Thanks @aekiss for the suggestions. I have incorporated all the suggestions except duplicating and modifying these lines of DATM caps, since a branch name is required to modify CDEPS code. Could you please advise in which branch I need to make these changes?

@aekiss
Copy link
Contributor

aekiss commented Jul 12, 2023

Ah, good point.

At some later stage we might want to have our own fork of CDEPS and use that as a submodule in ACCESS-OM3 so we can push our own branches there.

But for now at this early testing stage I think you should copy atm_comp_nuopc.F90 to CDEPS/extra_sources and change CDEPS/CMakeLists.txt to use this copy instead of the original.

@aekiss
Copy link
Contributor

aekiss commented Jul 12, 2023

Thanks @ezhilsabareesh8

FYI the CI compile test seems to sometimes fail with core dump but when restarted it's usually ok

@ezhilsabareesh8
Copy link
Contributor Author

Thanks @aekiss. I have updated atm_comp_nuopc.F90 and included it in CDEPS/extra_sources. CDEPS/CMakeLists.txt is updated to use CDEPS/extra_sources/atm_comp_nuopc.F90 instead of the original.

@aekiss
Copy link
Contributor

aekiss commented Jul 13, 2023

I've just pushed some minor tweaks.

More changes are needed in atm_comp_nuopc.F90 - @ezhilsabareesh8 can you please search for "jra" and duplicate and modify the relevant code to support a new datamode 'JRA55do' that calls your JRA55-do subroutines?

@ezhilsabareesh8
Copy link
Contributor Author

Thanks @aekiss, datamode JRA55do is added to atm_comp_nuopc.f90 and the relevant code is modified to support JRA55do

@aekiss aekiss merged commit 2b52198 into master Jul 19, 2023
aekiss added a commit to ACCESS-NRI/access-om3-configs that referenced this pull request Jul 19, 2023
Supports JRA55-do COSIMA/access-om3#45
and uses latest spack dependencies COSIMA/access-om3#48
@micaeljtoliveira micaeljtoliveira deleted the Support-JRA55-do branch July 19, 2023 22:43
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