Support vertical regridding for same grid with different alignments (Task 3 and 4 of 8)#4409
Closed
Support vertical regridding for same grid with different alignments (Task 3 and 4 of 8)#4409
Conversation
…_in_extension_transform_4209
…_in_extension_transform_4209
…dle_in_extension_transform_4209 Limit fieldbundle type for extension transform subtypes
…4232) * - Moved get_mapl_geom from GeomIO/SharedIO.F90 to GeomManager - Added MAPL_GeomGet to retrieve decomposition topology in case of a CubedSphere grid * Moved geom_get to its own file GeomGet.F90 * Added test for MAPL_GeomGet, activated test for MAPL_GridGet * Minor formatting
…t_name Implement & test EXPORT_NAME column for ACG3
…l spec is to be renamed while being re-exported
…re/pchakrab/user-routine-instrumentation
# Conflicts: # shared/CMakeLists.txt # shared/tests/CMakeLists.txt
…hakrab/user-routine-instrumentation
…rnal-new-name Specify new name for an internal spec when re-exporting
…hakrab/user-routine-instrumentation
2. Changed some logger info calls to debug
* Add vector basis kind support for regridding This implements issue #4376 by introducing VectorBasisKind to distinguish between NS (North-South/geographic) and GRID (grid-relative) vector components during regridding operations. Key changes: - New VectorBasisKind enum (NS, GRID, INVALID) in esmf_utils - String-based VariableSpec API: vector_basis_kind='NS' or 'GRID' - Field bundle metadata storage via FieldBundleSet/Get - Regridder integration: passes basis kind directly to get_basis() - Validation ensures vector_basis_kind only used with VECTOR bundles - get_basis() refactored with early returns (no nesting) - Comprehensive test suite (12 tests covering all scenarios) User API supports vector_basis_kind='NS' (default for vectors) or 'GRID'. The enum-based internal API ensures type safety throughout the stack. * Fix unallocated vector_basis_kind bugs - VariableSpec: Check allocation before accessing vector_basis_kind - VariableSpec: Default to VECTOR_BASIS_KIND_NS when unallocated - FieldBundleCreate: Auto-set NS basis for VECTOR bundles - Remove unused status variable in verify_deferred_items * Use itemType to detect vectors in VariableSpec Addresses reviewer feedback to use the canonical itemType field instead of heuristics (vector_component_names, standard_name format) to determine if a VariableSpec represents a vector. Changes: - VariableSpec: Check itemType == MAPL_STATEITEM_VECTOR - Removed complex heuristic logic and unused is_vector variable - Test_VectorBasisKind: Added itemtype parameter and import * Add vector_basis_kind for VectorBracket - Add vector_basis_kind support to VectorBracketClassAspect - Generalize VectorBracket regridding for any even number of fields - Simplify vector_basis_kind validation in VariableSpec - Make vector_basis_kind non-allocatable (always has default value) * Fix constant name: VECTORBRACKET not VECTOR_BRACKET Rename MAPL_STATEITEM_VECTOR_BRACKET to MAPL_STATEITEM_VECTORBRACKET for consistency with naming convention MAPL_STATEITEM_<class> where the class is VectorBracket (no underscore in compound name). * Add default for missing vector_basis_kind When vector_basis_kind metadata is not present in a bundle, use VECTOR_BASIS_KIND_NS as the default instead of failing. This makes the code more robust and consistent with how other optional metadata attributes are handled.
) * Add descriptive failure messages for MAPL3G_Comp_Test_case* tests - Parse test_case_descriptions.md at CMake configure time - Pass descriptions to run_comp_tester.cmake as parameter - Enhance run_comp_tester.cmake to display: - Step progress during test execution - Test description and specific step on failure - Eliminates duplication by using single source of truth Fixes #4390 * Spelling fix --------- Co-authored-by: Matt Thompson <matthew.thompson@nasa.gov>
…ugs (#4397) - Added 4 integration tests for vector_basis_kind feature to verify regridding uses basis correctly - Fixed 5 pre-existing bugs in GridVectorBasis implementation that prevented it from working - Fixed naming inconsistency: FIELDBUNDLETYPE_VECTOR_BRACKET -> FIELDBUNDLETYPE_VECTORBRACKET Bugs fixed: 1. Missing allocation of basis%elements in new_GridVectorBasis.F90 2. Wrong dimensions in grid_get_corners.F90 (used size() instead of exclusiveCount) 3. Array slice typo in new_GridVectorBasis.F90 (j+j+1 -> j:j+1) 4. Missing default basis for VECTORBRACKET in FieldBundleCreate.F90 5. Wrong bundle creation in Regridder.F90 (ESMF_FieldBundleCreate -> MAPL_FieldBundleCreate) Tests added: - test_regrid_vector_with_ns_basis: Explicitly set NS basis and regrid - test_regrid_vector_with_grid_basis: Explicitly set GRID basis and regrid - test_regrid_vector_default_basis: Verify default is NS - test_regrid_vector_bracket_with_basis: Test VectorBracket with basis kind Addresses #4394
…te key to the merged hconfig
…ate in V ariableSpec. A permament fix would involve adding allocation_status and has_deferred_aspects to the ESMF_State object's info object
# Conflicts: # AGENTS.md
Use ESMF_HConfigAdd, instead of ESMF_HConfigSet
…-state Temporary fix for StateClassAspect, that provides support for ESMF_St…
This commit implements Tasks 1 and 2 from the vertical alignment feature: - Add coordinate_direction to VerticalGrid with VerticalCoordinateDirection type - Add vertical_alignment to VariableSpec and VerticalGridAspect New Features: - VerticalCoordinateDirection enum-like type with constants for upward/downward - VerticalAlignment enum-like type with constants for with_grid/upward/downward - Default grid coordinate_direction is 'downward' (GEOS convention) - Default field vertical_alignment is 'with_grid' (follows grid direction) - Supports shortcuts 'U'/'D' for upward/downward in string constructors - VerticalAlignment.resolve() method converts alignment to coordinate direction Implementation: - Updated VerticalGrid base class and all implementations (BasicVerticalGrid, FixedLevelsVerticalGrid, ModelVerticalGrid) - Added vertical_alignment field to VariableSpec (user-facing string interface) - Updated VerticalGridAspect to use typed VerticalAlignment internally - Added comprehensive unit tests for both coordinate_direction and vertical_alignment Testing: - All 44 vertical_grid tests pass on NAG compiler - All 44 vertical_grid tests pass on GFortran compiler - New tests verify default values, get/set operations, and string conversions Related to #4377
Simplify the VerticalRegridTransform constructor by grouping related configuration parameters into a VerticalRegridParam type, reducing the parameter count from 10 to 5. Changes: - Add VerticalRegridParam type with fields: stagger_in, stagger_out, method, src_alignment, dst_alignment, is_degenerate_case - Refactor new_VerticalRegridTransform() to accept regrid_param instead of individual optional parameters - Update VerticalGridAspect to populate and pass VerticalRegridParam - Add Test_VerticalRegridTransform.pf with degenerate case tests - Both vertical coordinate fields (v_in_coupler, v_out_coupler) remain as direct parameters since they are time-varying runtime data This follows MAPL's existing RegridParam convention for horizontal regridding and makes future extensions easier without changing the constructor signature.
Implement degenerate case handling in vertical regridding where source and destination share the same vertical grid but have different field alignments (e.g., one UP, one DOWN). This is an optimization that uses simple array operations instead of full regrid matrix computation. Task 3: Add VerticalAlignment to Field/FieldBundle API - Move VerticalAlignment from generic3g/specs to vertical_grid module - Add vert_alignment parameter to Field/FieldBundle create, get, and set - Store alignment in ESMF_Info metadata with KEY_VERT_ALIGNMENT - Export alignment types through VerticalGrid_API Task 4: Implement Degenerate Case in VerticalRegridTransform - Add VerticalRegridParam type to group regrid configuration - Refactor constructor to use VerticalRegridParam (reduces params 10→5) - Add src_alignment, dst_alignment, is_degenerate_case fields - Implement copy_field_() and copy_field_flipped_() for degenerate case - Update VerticalGridAspect to detect degenerate case and pass flag - Add Test_VerticalRegridTransform.pf with flip tests Key design decisions: - Vertical coordinate fields remain direct parameters (time-varying data) - Degenerate case detected by grid ID comparison + alignment check - Same alignment → direct copy, different alignment → vertical flip - Follows existing RegridParam convention for consistency Testing: - 422 generic3g tests pass (16 pre-existing scenario failures) - New tests verify DOWN→UP and UP→DOWN flips work correctly - 23 field_bundle tests pass, 20 field tests pass
4b29a47 to
c63adb6
Compare
Task 5: Full vertical regridding with alignment support for different grids Major changes: - Refactored VerticalRegridTransform::update() method to reduce nesting - Reduced from 3 levels to 1 level (75 → 50 lines) - Added type-bound procedures: process_field() and process_fieldbundle() - Moved degeneracy check to field level for better organization - Fixed alignment-based flipping logic - DOWN alignment: no flipping (default ESM orientation) - UP alignment: flip coordinates and data before/after interpolation - Removed incorrect assumptions about coordinate monotonicity - Added NullTransform optimization in VerticalGridAspect::make_transform() - Returns NullTransform when grids AND alignments match - Completely eliminates transform overhead for degenerate cases - Comprehensive test suite with 11 tests covering all alignment/monotonicity combinations: 1-2: Degenerate case (same grid, flip only) DOWN↔UP 3-5: Different grids with reversed coordinates and alignments 6-7: Different grids, same alignment (DOWN→DOWN) 8-11: All combinations to ensure no assumptions about coordinate monotonicity All tests passing with NAG compiler (11/11). Files modified: - generic3g/transforms/VerticalRegridTransform.F90 - generic3g/specs/VerticalGridAspect.F90 - generic3g/tests/Test_VerticalRegridTransform.pf - .opencode/skills/mapl-build/SKILL.md (updated test workflow)
- Add vertical_alignment field to ExtDataRule type - Parse vertical_alignment from YAML configuration - Update implementation plan to mark Task 6 complete Enables ExtData YAML files to specify vertical alignment: vertical_alignment: upward # or "downward" or "with_grid" The field is available in ExtDataRule for future integration with field configuration and regridding operations.
c63adb6 to
b3b92dc
Compare
Collaborator
Author
|
Closing this PR as it has been superseded by #4410, which contains Tasks 3-6 together. |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement degenerate case handling in vertical regridding where source and destination share the same vertical grid but have different field alignments (e.g., one field aligned UP, another DOWN). This optimization uses simple array operations instead of full regrid matrix computation.
Changes
Task 3: Add VerticalAlignment to Field/FieldBundle API
generic3g/specstovertical_grid(proper location alongside VerticalGrid)vert_alignmentparameter toMAPL_FieldCreate(),MAPL_FieldGet(),MAPL_FieldSet()vert_alignmentparameter to FieldBundle create, get, and set operationsKEY_VERT_ALIGNMENTVALIGN_WITH_GRID,VALIGN_UP,VALIGN_DOWN) throughVerticalGrid_APITask 4: Implement Degenerate Case in VerticalRegridTransform
VerticalRegridParamtype: Groups regrid configuration (stagger_in, stagger_out, method, src_alignment, dst_alignment, is_degenerate_case)regrid_paraminstead of 6 individual parameters (reduces parameter count 10→5)VerticalGridAspect%matches()now checks both grid ID and resolved alignmentcopy_field_()(direct copy)copy_field_flipped_()(vertical flip:x_out(:,k,:) = x_in(:,nlev-k+1,:))VerticalGridAspect%make_transform()detects degenerate case and populatesVerticalRegridParamTest_VerticalRegridTransform.pfwith two degenerate case tests verifying DOWN→UP and UP→DOWN flipsDesign Decisions
Why keep coordinate fields as direct parameters?
Vertical coordinate fields (
v_in_coupler,v_out_coupler) remain as direct constructor parameters because they are time-varying runtime data objects (e.g., pressure levels), not static configuration. This differs from horizontal regridding where coordinates are static.Degenerate case detection
grid%matches()for BasicVerticalGrid)Follows MAPL conventions
The
VerticalRegridParamdesign mirrors the existingRegridParampattern used for horizontal regridding, providing consistency across the codebase.Testing
Related Issues
Part of vertical alignment implementation (Tasks 3 and 4 from implementation plan)