MetOffice / MetOffice/lfric_apps

Enable SURF Snow Analysis Ancillary with fd_start_dump Initialisation

Open
#792 0 comments 0 reactions 1 assignee Claimed by @rsdavies View on GitHub
enhancement
Dominant language
Fortran
Stars
31
Forks
118
Avg merge
2d 4h
Merged PRs (30d)
8

Description

### Are there any linked Issues or Pull Requests?

_No response_

### Brief description

#### Current Limitation

The SURF snow analysis ancillary can only be read in the following configuration:
- `init_option=checkpoint_dump`
- `checkpoint_read=.false.`

This prevents using SURF snow analysis with cold starts from a UM dump file (`fd_start_dump`), which is useful for testing or cold-starting a workflow.

It should possible (according to logicals in the code) to force the reading of the SURF snow analysis ancil with the following configuration, but this reveals a bug:
- `init_option=fd_start_dump`
- `checkpoint_read=.false.`
- `ancil_option = updataing`

#### Bug
The `fd_start_dump` and the SURF snow analysis ancil have identical variable names for their shared variables.
This means a shared reference conflict occurs when the same field exists in two collections:

1. SURF analysis snow fields (in ancil_fields):
- Registered with time_axis machinery
- Code correctly appends `_data` suffix for XIOS reads
- Field names: `tile_snow_mass_in_data`, `tile_snow_rgrain_in_data`, etc.

2. fd_start_dump snow fields (in fd_fields):
- Registered without time_axis
- Should not append `_data` suffix
- Problem: Field names also use `_in` suffix (accidentally copied from analysis fields)

When both registrations use the same base field name (i.e. `tile_snow_mass_in`) in the shared depository, the second registration overwrites field read metadata. This causes a cascade failure:
- Time-varying read via time_axis correctly sets `can_read()=true` with `_data` appended
- Subsequent attempt to read non-time-varying ancils attempts to read `tile_snow_mass_in` (without `_data`)
- XIOS fails: `tile_snow_mass_in does not exist in object`

#### Why This Wasn't Caught

The rose-stem test suite only covers `init_option=checkpoint_dump` with `checkpoint_read=.false.`, which avoids triggering `fd_start_dump` snow field registration. The bug emerges when combining:
-`init_option=fd_start_dump` (cold start from UM dump)
- `ancil_option=updating` (read ancillaries)

#### Solution

Resolve the shared reference conflict by disambiguating field names based on source:

1. `fd_start_dump` snow fields: Use base names (remove `_in` suffix)
- Reflects that these come from the dump, not an ancillary input
- Example: `tile_snow_mass`, `tile_snow_rgrain`, `n_snow_layers`

2. SURF analysis snow fields: Use `_analysis` suffix (rename from `_in`)
- Explicitly indicates these are from SURF analysis data
- Example: `tile_snow_mass_analysis`, `tile_snow_rgrain_analysis`

3. Field name mapping:

Field | fd_start_dump | SURF Analysis
---------------------- | ----------------------- | -------------------------
Snow mass | tile_snow_mass | tile_snow_mass_analysis
Grain radius | tile_snow_rgrain | tile_snow_rgrain_analysis
Snow layers | n_snow_layers | n_snow_layers_analysis
Snow depth | snow_depth | snow_depth_analysis
Snow under canopy | snow_under_canopy | snow_under_canopy_analysis
Snowpack density | snowpack_density | snowpack_density_analysis

#### Implementation

Code Changes:
- `create_fd_prognostics_mod.f90`: Rename field setup names to base names
- `process_inputs_alg_mod.x90`: Update field retrieval logic for both sources
- `gungho_init_fields_mod.X90`: Update field cleanup to match

XIOS Definition Changes:
- Update `fd_start_dump` snow field definitions (base names)
- `file_def_ancil_main.xml`: Update SURF analysis snow field definitions (`_analysis` suffix)

#### Testing & Validation

Regression Testing:
- Verify existing rose-stem test (`init_option=checkpoint_dump`, `checkpoint_read=.false.`) produces identical results (bit-for-bit or within acceptable tolerance)

New Test Coverage:
- `fd_start_dump + ancil_option=fixed + snow_source='surf'`
Validates snow initialization from SURF analysis for short runs

- `fd_start_dump + ancil_option=updating + snow_source='surf'`
Validates snow initialization and daily time-axis updates

Test Scenarios Matrix:

Scenario | init_option | ancil_option | snow_source | Expected | Status
----------|-----------------|--------------|-------------|----------|------------------
Existing | checkpoint_dump | — | surf | Works | Must remain unchanged
New 1 | fd_start_dump | fixed | surf | Works | Static snow for short runs
New 2 | fd_start_dump | updating | surf | Works | Dynamic snow for DA cycles
Baseline | fd_start_dump | fixed | start_dump | Works | Unchanged, no SURF

#### Benefits

- Enables SURF snow analysis for workflows with cold starts and in testing with a um2lfric dump
- Fixes latent bug that could affect future initialisation configurations
- Improves code clarity: field names now explicitly indicate source
- Maintains backward compatibility for existing non-SURF workflows

#### Notes

- Snow analysis is a "special case" ancillary that updates daily (unlike for example, aerosols), making it suitable for use as a static ancil in short runs
- Checkpoint dump fields use a separate initialization path and are not affected by these changes
- I added the `_in` suffix when adding this support initially, precisely to avoid this sort of issue. By making it much clearer that this is the *analysis* field it should prevent confusion in future.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.