Skip to content

Conversation

@ogurses
Copy link
Collaborator

@ogurses ogurses commented Oct 29, 2025

No description provided.

ogurses and others added 17 commits October 15, 2024 15:08
Define new variables to track tracer changes
due to advection and diffusion.

We want to save for now diffusion and advection
contribution to the tracer changes. Horizontal and
vertical diffusion includes Redi
parametrization (if it is set .true.).
Fill __ciso directive to ensure that
carbon isotope code works. Medusa interface is
added.
Use __coccos in src/CMakeLists.txt to
include 2 more phytoplankton classes

FESOM2.6-REcoM setup with 4 phytoplankton,
3 zooplankton and 2 detritus functional types.

BREAKING CHANGE: FESOM2.6-REcoM setup with 4p3z2d
Correct temperature function for small phytoplankton and diatoms.

Previous update for the fouth phytoplankton class and
new temperature functions for small phytoplankton and diatoms
resulted in very low Phaeocystis biomass.
We recognised that this is related to missing code lines in the
temperature function which we correct with this update.
…ostics

Use namelist.io to activate diagnostics.

Sinking velocity of particle (fast and slow) are calculated but not written into a file before.
We fix the  missing diagnostics for sinking velocity of detritus classes. Besides we
Extracted the K0, solubility  which is computed
in gasx.F90.
We add __coccos directive to get the correct temperature
function in silicate assimilation.

In the coccos and Phaeocystis version, it should be dependent on the new temperature function, Temp_diatoms.
Use the original Arrhenius function in simpler phytoplankton version. Besides, light limitation diagnostics is corrected.
It does affect only the diatoms related output.
Correct reported salt flux bug in oce_ale_tracer.F90.
#721
We commented out the linear model where degradation is directly
proportional to light intensity and the chlorophyll degradation
rate is capped at a maximum of 0.3. Reason for this is the unrealistic
air2sea co2 fluxes in the 2p3z2d setup.
The reactivated saturation model is more realistic. It accounts for light
saturation effects.
At low light, degradation increases roughly linearly with PAR
At high light, the degradation rate approaches an asymptotic maximum
…tives with namelist parameters

Replace preprocessor directives with namelist parameters:
* `__3Zoo2Det` → `enable_3zoo2det`
* `__coccos` → `enable_coccos`

* Enhanced inline comments with ecological significance and biological context
*Complete variable/parameter documentation with units and descriptions
* Clear sectioning with visual separators and variable definitions
* Equation explanations showing mathematical relationships and biological processes
* Logic documentation for decision trees and conditional blocks
* Comments now explain "why" not just "what"

**Sequence Validation**
* Generates expected tracer ID list based on configuration
* Shows exact ID order and position-by-position comparison

**Configuration-Specific Clash Detection**
* Full Model (both flags): Don't use IDs 1023-1024
* Coccos-only: Use 1023-1028 (NOT 1029-1034)
* 3Zoo2Det-only: Use 1029-1030 for microzoo (NOT 1035-1036)

**Validation Checks**
* Exact sequence matching against expected IDs
* Duplicate ID detection
*Forbidden ID detection per configuration

BREAKING CHANGE: replace  directives with namelist parameter for model configuration
@JanStreffing JanStreffing added this to the FESOM 2.7 milestone Oct 29, 2025
@JanStreffing JanStreffing changed the title Fesom2.6 recom tra diags Fesom2.6 recom tra diags and CI test Oct 29, 2025
@JanStreffing JanStreffing linked an issue Oct 29, 2025 that may be closed by this pull request
@ogurses ogurses requested a review from suvarchal October 29, 2025 14:34
@ogurses
Copy link
Collaborator Author

ogurses commented Oct 29, 2025

CI test for recom is on the way. Any help is welcome @suvarchal

@JanStreffing
Copy link
Collaborator

! O:G
! Save horizontal and vertical advective fluxes.
! We have the values on the nodes
! We do not know how much each edge contributes
! to the nodes it connects
! Notes from Patrick: del_ttf includes
! Low-order solution. But, del_ttf_advhoriz and
! del_ttf_advvert contain antidiffusive fluxes 
! from the FCT scheme

!if (.FALSE.) then
! O:G - tra_diag
!#if defined (__recom)
!        if (tracers%data(tr_num)%ltra_diag) then
!           do n=1, myDim_nod2D+eDim_nod2D
!              nu1 = ulevels_nod2D(n)
!              nl1 = nlevels_nod2D(n)
!              do nz = nu1, nl1-1
                 ! Horizontal advection part
!                 tracers%work%tra_advhoriz(nz,n,tr_num) = tracers%work%del_ttf_advhoriz(nz,n)
                 ! Vertical advection part
!                 tracers%work%tra_advvert (nz,n,tr_num) = tracers%work%del_ttf_advvert(nz,n)
!              end do
!           end do
!        end if
!#endif
!endif 

Still needed @ogurses?

!#if defined (__recom) ! not necessarily should belong to recom case
! tracers%work%tra_advhoriz = 0.0 ! O:G - tra_diag
! tracers%work%tra_advvert = 0.0
ttf_rhs_bak = 0.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Has no impact on CI test, so probably ok. Just flagging to be checked by @patrickscholz

@suvarchal
Copy link
Collaborator

CI test for recom is on the way. Any help is welcome @suvarchal

last-minute working on compute time reporting and proposal, can take a look tomorrow

- Add namelist.recom.working and namelist.tra.working to test/input/recom/
- Based on branch owner's working versions with paths fixed for CI
- bgc_num=22 for 2p1z1d configuration
- REcoM_restart=false for initialization from climatology
- Avoids any runtime sed issues that might cause parsing errors
- mkrun does NOT create namelist.recom or namelist.tra for REcoM
- Copy from config/bin_2p1z1d/ and patch all file paths
- Replace individual filenames in namelist.tra with our test files
- Fix REcoM_restart=false and REcoMDataPath='./'
- This should finally work\!
Line 178 was incorrectly reading 'pavariables' namelist when it should
read 'parecomsetup', causing pavariables to be read twice. This resulted
in 'Invalid line in namelist' and EOF errors when trying to run REcoM.

Fixed by correcting the namelist read on line 178 from pavariables to
parecomsetup, so the reading order matches the namelist file structure.
The code doesn't read nam_rsbc (no variables defined), so remove it
from the namelist to avoid parsing errors. This also simplifies the
workflow - we only need to patch REcoM_restart and tracer files now.
The 2p1z1d (22 tracers) configuration causes segfaults during initialization,
likely due to arrays (wFluxPhy, wFluxDia, etc.) not being properly allocated
for the simplified configuration.

Use the full 2p3z2d (30 tracers) configuration which is better tested and
has all arrays properly allocated.
The wFlux* arrays (wFluxDet, wFluxPhy, wFluxDia, wFluxCocco, wFluxPhaeo)
and their global counterparts (GlowFlux*) were being initialized to 0.0
but were never allocated, causing a segmentation fault at line 137.

Added allocate() statements for all missing arrays before initialization.
This bug affected all REcoM configurations (2p1z1d and 2p3z2d).
- Changed initial year from 1958 to 1948 in clockinit
- Updated REcoM forcing year range to 1948
- Added copying of 1948 forcing files in CI workflow

The test forcing data files are from 1948, so we need to start
the simulation in 1948 rather than 1958.
Only trigger REcoM CI test on pull requests to main branch
or manual workflow_dispatch. This reduces CI load during
development.
The CORE forcing data uses noleap calendar (365 days/year).
Set include_fleapyear=False to match the forcing data calendar.
The monthly_event subroutine requires two arguments (do_output, N),
but was called with only one at line 1872 in gen_surface_forcing.F90.
This caused undefined behavior when accessing the uninitialized N
parameter, leading to a segmentation fault during forcing updates.

Fixed by adding N=1 (check every month) as the second argument.
The code has a hardcoded default nm_fe_data_file='DustClimMonthlyAlbani.nc'
but our test file is named 'DustClimMonthlyAlbani_pimesh.nc'.

Since we remove the nam_rsbc section (it's not recognized by the Fortran
namelist parser), the code uses the default filename. Create a symlink
so the file is accessible under both names.
The dust forcing file is read from ClimateDataPath (test/input/recom/),
not from the working directory. Create the symlink in the correct location.
The RiverFe array was declared in recom_modules.F90 but never allocated,
causing a segmentation fault at line 2080 in gen_surface_forcing.F90
when trying to set RiverFe = 0.0d0.

Added allocation and initialization for RiverFe alongside the other
river nutrient arrays.
Added mslp (mean sea level pressure) forcing configuration to enable
CO2 flux calculations in REcoM. Without atmospheric pressure, the CO2
flux calculation fails with NaN values.

Changes:
- Added namelist.forcing section with l_mslp=True
- Configured to read from 'slp.' files with 'slp' variable
- Copy PHC2_salx.nc and runoff.nc to work directory in CI
Add preprocessing step to process REcoM input NetCDF files using
'cdo copy' (with ncks as fallback) to add proper _FillValue attributes.

Files processed:
- oxy5deg.nc
- si5deg.nc
- talk5deg.nc
- din5deg.nc
- GLODAPv2.2016b.PI_TCO2_fesom2_mmol_fix_z_Fillvalue.nc

This ensures proper land masking and interpolation instead of using
dummy fill values (10000000000.0 and 0.0).

Suggested by colleague to fix interpolation issues.
Processed REcoM input files with 'cdo copy' to add proper fill values:
- oxy5deg.nc
- si5deg.nc
- talk5deg.nc
- din5deg.nc
- GLODAPv2.2016b.PI_TCO2_fesom2_mmol_fix_z_Fillvalue.nc

This is a one-time fix to the repository files, removing the need
for runtime preprocessing in the CI workflow.
The default namelist.forcing.CORE2 already has mslp (atmospheric
pressure) configured but disabled (l_mslp=.false.). Patch it to
l_mslp=.true. in the workflow to enable reading of slp.*.nc files.

This is needed for REcoM CO2 flux calculations which require
atmospheric pressure (Patm).

Removed unnecessary namelist.forcing section from setup.yml since
we just patch the default config file directly.
The sed command wasn't matching because dots are regex wildcards.
Escaped the dots in .false. to match the literal string.

Now l_mslp=.false. will correctly be replaced with l_mslp=.true.
Show l_mslp status in the test run output to diagnose why
atmospheric pressure is still not being read (Patm=0.0).
The namelist.forcing file has:
   l_mslp = .false.  (with spaces)

Previous pattern l_mslp=\.false\. didn't match.
New pattern l_mslp *= *\.false\. matches optional spaces.

This should finally enable atmospheric pressure forcing\!
Namelists are working correctly now, so removed verbose dumps.
Kept minimal verification that patches were applied:
- REcoM_restart setting
- l_mslp atmospheric pressure flag

This will make the CI logs much cleaner and easier to read.
Problem: Fortran NetCDF library and CDO/NCO tools use different
fill value conventions, causing NaN values in tracer initialization.

Solution: Make fill value detection more robust:
1. Try _FillValue attribute (Fortran NetCDF standard)
2. Fall back to missing_value attribute (used by CDO/NCO)
3. Use NetCDF default fill value (9.969e+36) if neither exists
4. Broadcast fill value to all MPI ranks

This fixes the 'REcoM_DIC: NaN' error during initialization by
properly handling land/missing values in input NetCDF files
created with different tools.

Fixes issue reported by colleague about incompatible fill value
conventions between different NetCDF tools.
Added diagnostic output to track where DIC becomes NaN:
- Check state(idic) and sms(idic) before DIC calculation
- Check DIC after calculation
- Print node, depth, lon/lat location when NaN detected
- Stop execution immediately to get clean traceback

This will help identify whether:
1. The state already contains NaN (initialization issue)
2. The SMS contains NaN (biogeochemistry issue)
3. The addition creates NaN (numerical issue)

The error occurs at lon=-43.88, lat=14.55 consistently, so
this should pinpoint the exact cause.
Added diagnostic checks at the point where DIC is read from state
array and converted for CO2 flux calculation.

Since the SMS debug didn't trigger, the NaN must appear between
SMS calculation and forcing. This will catch:
1. If state(one,idic) is already NaN when forcing starts
2. If the conversion to REcoM_DIC creates NaN

Will print node number, coordinates, and related state values
to identify the exact source of NaN.
@JanStreffing JanStreffing force-pushed the fesom2.6_recom_tra_diags branch from 619ee7c to c6c21e0 Compare November 12, 2025 22:28
@JanStreffing
Copy link
Collaborator

A couple of interesting changes so far. Some of the recom code was missing allocations, that triggered GNU errors. I am left with NANs in one init field that I failed to resolve.

@ogurses, @suvarchal: Can you check this out?

@JanStreffing JanStreffing modified the milestones: FESOM 2.7, FESOM 2.7.1 Nov 17, 2025
When ballasting is used, vertical sinking is adjusted
if the detritus carbon biomass exceeds a treshold value (0.001).
This results in large air-sea co2flux uptake. Weremove this constraint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create REcoM CI test

5 participants