Skip to content

Multiple nesting feature#13

Merged
rafa-guedes merged 10 commits into
rom-py:mainfrom
MireyaMMO:multiple-nesting-feature
Apr 15, 2026
Merged

Multiple nesting feature#13
rafa-guedes merged 10 commits into
rom-py:mainfrom
MireyaMMO:multiple-nesting-feature

Conversation

@MireyaMMO

Copy link
Copy Markdown
Contributor

Implementation Summary: Multiple Child Nests Support for ROMPY-SWAN

Overview

Successfully implemented support for defining multiple child nested grids from a single parent grid in ROMPY-SWAN, addressing a key limitation where only one child nest was previously allowed.

Changes Made

1. New NEST Component (output.py)

  • Created a new NEST component that couples NGRID and NESTOUT components together
  • Automatically synchronizes sname across all three components (NEST, NGRID, NESTOUT)
  • Supports both structured (NGRID) and unstructured (NGRID_UNSTRUCTURED) nested grids
  • Handles dict-based initialization with automatic sname propagation

Key Features:

nest = NEST(
    sname="child_1",
    ngrid=dict(model_type="ngrid", grid={...}),
    nestout=dict(fname="nestout_child1.swn", times={...}),
)

2. Updated OUTPUT Component (group.py)

  • Added new nests field that accepts a list of NEST components
  • Maintained backward compatibility with deprecated ngrid and nestout fields
  • Auto-converts legacy single nest configurations to the new format with deprecation warning
  • Validates that all nest snames are unique
  • Prevents mixing of old and new approaches

New Usage:

output = OUTPUT(
    nests=[
        dict(sname="child_1", ngrid={...}, nestout={...}),
        dict(sname="child_2", ngrid={...}, nestout={...}),
    ],
)

3. Updated OutputInterface (interface.py)

  • Modified time_interface validator to handle nests properly
  • Skips deprecated nestout field to avoid triggering deprecation warnings
  • Iterates through nests list to set time parameters for each nest's NESTOUT component
  • Maintains functionality while eliminating unnecessary deprecation warnings

4. Validators and Error Handling

  • Unique snames: Ensures each nest has a unique identifier
  • No mixing: Prevents combining nests with legacy ngrid/nestout fields
  • Automatic conversion: Legacy configurations are automatically converted to new format
  • Deprecation warnings: Clear warnings guide users to migrate to the new approach

Testing

Comprehensive test suite integrated into existing test files:

  • test_nest: Basic NEST component rendering
  • test_nest_sname_sync: Validates automatic sname synchronization
  • test_output_multiple_nests: Tests multiple nests in OUTPUT
  • test_output_nests_unique_snames: Validates unique sname requirement
  • test_output_cannot_mix_nests_and_legacy: Ensures old/new approaches don't mix
  • test_output_group_all_set: Tests legacy backward compatibility
  • test_output_group_all_set_with_nests: Tests new nests approach
  • All 202 existing tests pass: Full backward compatibility maintained

Usage Example

YAML Configuration

output:
  nests:
    - sname: child_1
      ngrid:
        model_type: ngrid
        grid: {xp: 167.0, yp: -45.5, xlen: 0.1, ylen: 0.1, mx: 12, my: 14}
      nestout:
        fname: nestout_child1.swn
        times: {tfmt: 1, dfmt: hr}
    
    - sname: child_2
      ngrid:
        model_type: ngrid
        grid: {xp: 166.0, yp: -46.5, xlen: 0.1, ylen: 0.1, mx: 8, my: 6}
      nestout:
        fname: nestout_child2.swn
        times: {tfmt: 1, dfmt: hr}

@rafa-guedes

Copy link
Copy Markdown
Contributor

@MireyaMMO this is great, thanks for sending this pull request :)

Sorry for taking a bit to get back on this, a bit busy at the moment, but I'll address this soon.

@rafa-guedes

Copy link
Copy Markdown
Contributor

@MireyaMMO thank you so much for this this contribution — multiple nest support is a great new feature and the overall approach is solid. I've made some improvements on top of your code before merging, described below. I'm sorry this took a bit long to be reviewed.

interface.py

Extended OutputInterface to inject runtime times into nestout components within each NEST, consistent with how other write components are now handled.

output.py — NEST design

Simplified the validator logic in NEST. Rather than using a mode-before validator to inject sname into child dicts, we give sname a default of "nest" on both NGRID and NESTOUT directly — this makes behaviour consistent whether children are passed as dicts or pre-built objects, and removes the need for the injection step entirely.

We also defined a shared SNAME_TYPE to avoid repeating field constraints across BaseLocation, BaseWrite, and NEST, and added min_length=1 to both base classes.

group.py

Split the nest_or_ngrid_and_nestout validator into two with clearer responsibilities:

  • migrate_legacy_ngrid_nestout — handles backward compatibility only, marked for future removal.
  • nests_snames_unique — permanent uniqueness check on the nests list.

Tests

  • Consolidated duplicate tests and updated remaining ones to use the new nests API.
  • Added a test that the legacy ngrid/nestout path emits a DeprecationWarning.
  • Added tests verifying that OutputInterface correctly injects runtime times into nest components.

Docs

Updated docs/components/output.md to introduce NEST as the preferred approach and mark NGRID, NESTOUT standalone usage as deprecated.

@rafa-guedes

Copy link
Copy Markdown
Contributor

The only issue though is that there were some conflicts with changes merged to main after you started your work. I ended up doing the following: pulled your branch locally, merged the latest changes from main into it to bring it up to date, resolved the conflicts, and then made the changes described above. The updated code is on review/multiple-nests on the main repository — if you pull from there and push to your branch, your PR will be ready to merge:

git fetch upstream
git checkout multiple-nesting-feature
git reset --hard upstream/review/multiple-nests
git push origin multiple-nesting-feature --force-with-lease

Alternatively, I'm also happy to open a new PR directly from review/multiple-nests — all your original commits are included and you'll be fully credited for the work. Just let me know which you'd prefer.

@MireyaMMO

Copy link
Copy Markdown
Contributor Author

Brilliant @rafa-guedes the improvements make a lot of sense!

I have just followed your steps provided in the comment above and there are no more conflicts with main.

@rafa-guedes rafa-guedes merged commit 3428349 into rom-py:main Apr 15, 2026
2 checks passed
@MireyaMMO MireyaMMO deleted the multiple-nesting-feature branch April 15, 2026 04:09
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.

2 participants