Skip to content

Fix ESMF encode truncating the fill value into a valid node index - #1744

Open
cmdupuis3 wants to merge 1 commit into
mainfrom
cmd/esmf_encode_fill_overflow
Open

Fix ESMF encode truncating the fill value into a valid node index#1744
cmdupuis3 wants to merge 1 commit into
mainfrom
cmd/esmf_encode_fill_overflow

Conversation

@cmdupuis3

@cmdupuis3 cmdupuis3 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Closes #1745

Overview

_encode_esmf applies the UGRID→ESMF index offset to the whole face_node_connectivity array, padding included, and then encodes elementConn as int32. INT_FILL_VALUE + 1 is -2**63 + 1, and narrowing that to int32 truncates it to 1, so every padded slot of a ragged mesh is written to disk as node 0.

Nothing raises, and the resulting file is structurally valid, so any reader — including uxarray's own — takes those slots for real vertices. Faces gain spurious extra nodes and produce incorrect polygons. This is the case that originally surfaced this family of bugs.

The fix offsets only the valid indices and writes ESMF's own -1 into the padding, which is what the variable's _FillValue attribute already advertises. The test asserts on the encoded array and on what lands on disk after the int32 narrowing, rather than on a round trip: a round trip can hide an encode bug when the reader's inverse offset happens to cancel it.

PR Checklist

General

  • An issue is created and linked
  • Added appropriate labels (if your uxarray repo permissions allow it)
  • Filled out Overview and Expected Usage (if applicable) sections

Testing & Benchmarking

  • There is adequate test coverage of changes from this PR (add new tests if needed)

Documentation and Examples

  • Docstrings updated with any function changes, and included in all new functions

AI Disclosure

AI Usage: Claude Opus 5

  • I have tested and take responsibility for all AI-generated content in my PR.

`_encode_esmf` applied the 0-based to 1-based offset to the whole
connectivity array, padding included, and `elementConn` is encoded as
int32. `INT_FILL_VALUE + 1` is `-2**63 + 1`, which the narrowing truncates
to `1`: every padded slot lands on disk as node 0.

Nothing raises. The file is well formed, so any reader -- including this
one -- takes those slots for real vertices, giving faces spurious extra
nodes and incorrect polygons.

Offset only the valid indices and write ESMF's own -1 into the padding.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cmdupuis3 cmdupuis3 added bug Something isn't working accuracy Related to mathematical accuracy of operations, such as numerical precision or rounding errors labels Sep 10, 2026
@cmdupuis3 cmdupuis3 self-assigned this Sep 10, 2026
@cmdupuis3 cmdupuis3 changed the title Title: Fix ESMF encode truncating the fill value into a valid node index Fix ESMF encode truncating the fill value into a valid node index Sep 10, 2026
@cmdupuis3
cmdupuis3 marked this pull request as ready for review September 11, 2026 18:50
@cmdupuis3
cmdupuis3 requested a review from Sevans711 September 11, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accuracy Related to mathematical accuracy of operations, such as numerical precision or rounding errors bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix ESMF encode truncating the fill value into a valid node index

1 participant