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

Transient tracers bugfixed #659

Merged
merged 13 commits into from
Jan 14, 2025
Merged

Transient tracers bugfixed #659

merged 13 commits into from
Jan 14, 2025

Conversation

mbutzin
Copy link
Collaborator

@mbutzin mbutzin commented Dec 19, 2024

This branch includes bugfixes, missing and improved code and updated namelists to run FESOM-2.6 with transient tracers (14C, 39Ar, CFC-11, CFC-12, SF6).
Note that without / prior to these changes the model could be compiled without errors but the simulation would fail at runtime.

@JanStreffing
Copy link
Collaborator

There seems to be a problem with this branch right now. The tests fail with:

  560 |     use transit_bc_surface_interface
      |         1
Fatal Error: Cannot open module file 'transit_bc_surface_interface.mod' for reading at (1): No such file or directory
compilation terminated.

@mbutzin
Copy link
Collaborator Author

mbutzin commented Dec 19, 2024

There seems to be a problem with this branch right now. The tests fail with:

  560 |     use transit_bc_surface_interface
      |         1
Fatal Error: Cannot open module file 'transit_bc_surface_interface.mod' for reading at (1): No such file or directory
compilation terminated.

I removed L 560 which is obsolete now. Let me know if there is still a problem.

Copy link
Collaborator

@JanStreffing JanStreffing left a comment

Choose a reason for hiding this comment

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

Checks are all passing now. But there seem to be a few changes that look like they undo recent fixes unrelated to tracers. Some changes / undoing some of these will be needed.

@@ -2,8 +2,6 @@ module g_clock
!combining RT and Lars version
!
use g_config
use iso_fortran_env, only: error_unit
use mpi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this and other changes to gen_modules_clock intended?

src/io_meandata.F90 Show resolved Hide resolved
@@ -385,7 +377,7 @@ subroutine ini_mean_io(ice, dynamics, tracers, partit, mesh)
!_______________________________________________________________________________
! output surface forcing
CASE ('fh ')
call def_stream(nod2D, myDim_nod2D, 'fh', 'heat flux', 'W/m2', heat_flux_in(:), io_list(i)%freq, io_list(i)%unit, io_list(i)%precision, partit, mesh)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I seem to recall this was a recent fix, that seems to be made undone here. Probably a mistake.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Strange ... I did not touch gen_modules_clock.F90 and io_meandata.F90 at these lines.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It it probably because you based you changes upon an older version of fesom. I'd suggest you rebase your changes onto the latest main

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I run
esm_master install-awiesm-2.6
esm_master install-fesom-2.6
esm_master install-fesom-2.6-paleodyn
I always obtain io_meandata.F90 without these lines.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's because we download FESOM 2.6.1 right now via esm_tools. But here we are merging in the main branch. These lines have been additions to the main branch in the last few weeks. The strange thing for me, is that git removes them again in this PR, because you don't touch the same lines in your branch.

We can have a look together after the holidays to sort this out.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the status of this PR? Can I help with anything?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Something went wrong with branching off from the tag and merging back into main. For some reason doing so would remove the changes on main.

I suggest reimplementing in a branch off from main, and merging back into main.

@ackerlar
Copy link
Collaborator

ackerlar commented Jan 10, 2025

I tried to add the changes made to fix the transient tracers, based on the latest main version. @mbutzin could you have a look and try whether everything works for you https://github.com/FESOM/fesom2/tree/fix/tracers_in_main

I also created a PR #662


UPDATE:
I closed the above mentioned PR as the the changes were the same as in this PR here. Instead, I updated this branch and hope to address all required changes. @mbutzin please have a loot at the updated branch to see whether everything still works for you

@mbutzin
Copy link
Collaborator Author

mbutzin commented Jan 13, 2025

Please update (=replace) namelist.transit and namelist.io in the 'main' branch with the more recent versions from branch 'transient_tracers_bugfixed'. Otherwise the transient tracers will no longer work.

REAL(kind=WP) :: bc_surface
character(len=10) :: id_string
real(kind=WP), dimension(:), pointer :: a_ice
! real(kind=WP), dimension(:), pointer :: a_ice !! MB: where is this needed?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be commented out? Could be for a coupled setup you dont test. IMO not the right place to remove this line, even if it is not needed.

@ackerlar
Copy link
Collaborator

@mbutzin Do you mean those changes:


 &transit_param
+l_r14c  = .false.
+l_r39ar = .false.
+l_f11   = .false.
+l_f12   = .false.
+l_sf6   = .false.
 anthro_transit=.false.
 paleo_transit=.false.
 length_transit=1                ! 166 for anthro_transit=.true.
 ti_start_transit=1              ! 1 for D14C, 80 for CFC-12
-ifile_transit='/work/ollie/mbutzin/fesom2/input/trace_gases/Table_CO2_isoC_CFC12_SF6.txt'
+ifile_transit='/work/ab0246/a270108/fesom2_recom_config/input-for-awiesm/Table_CO2_isoC_CFCs1112_SF6.txt'
 r14c_a  = 1.0000  ! atm. 14C/C ratio, global mean
 r39ar_a = 1.0000       ! atm. 39Ar/Ar ratio, global mean
 xarg_a  = 9.34e-3      ! atm. Argon concn. (mole fraction), global mean

They are in the latest version of transient_tracers_bugfixed. The branch mentioned above (tracers_in_main) is not valid anymore. Sorry, if the EDIT is misleading.

@mbutzin
Copy link
Collaborator Author

mbutzin commented Jan 13, 2025

Exactly.

@ackerlar
Copy link
Collaborator

Okay, if otherwise everything looks good to you @mbutzin we can merge, can't we?

@mbutzin
Copy link
Collaborator Author

mbutzin commented Jan 13, 2025

Let me carry out another, hopefully final, test. From which branch should I pull the code now? I erroneously pulled from 'main' after the 404 of 'tracers_in_main'. Sorry for the confusion ...

@ackerlar
Copy link
Collaborator

from this one transient_tracers_bugfixed

@mbutzin
Copy link
Collaborator Author

mbutzin commented Jan 13, 2025

OK. I started a standalone test run at Levante, tomorrow we will know more.

@mbutzin
Copy link
Collaborator Author

mbutzin commented Jan 14, 2025

The results of the test run are OK. Go ahead, @ackerlar!

@ackerlar
Copy link
Collaborator

The results of the test run are OK. Go ahead, @ackerlar!

Great, thank you @mbutzin ! Amything else from your side @JanStreffing ? I think you would have to merge it

@JanStreffing JanStreffing merged commit 47d5228 into main Jan 14, 2025
4 checks passed
@JanStreffing JanStreffing deleted the transient_tracers_bugfixed branch January 14, 2025 16:17
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