Skip to content

mesh: reorder transfer_meshtags_to_submesh args to match create_submesh - #4518

Merged
jhale merged 1 commit into
mainfrom
jhale/mesh-review-transfer-meshtags-arg-order
Sep 18, 2026
Merged

jhale merged 1 commit into
mainfrom
jhale/mesh-review-transfer-meshtags-arg-order

Conversation

@jhale

@jhale jhale commented Sep 15, 2026

Copy link
Copy Markdown
Member

create_submesh returns (submesh, entity_map, vertex_map, geom_map), but
transfer_meshtags_to_submesh took (tags, submesh, vertex_map, cell_map)

  • vertex/cell swapped relative to how callers just received them, which
    was the natural mistake the F12-1 validation now catches. Reorders to
    (tags, submesh, cell_map, vertex_map) so create_submesh's result can be
    unpacked and passed straight through. Breaking change, no deprecation
    path.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com


Stack created with GitHub Stacks CLIGive Feedback 💬

@jhale
jhale added this pull request to stack #4520 September 15, 2026 09:18
@jhale
jhale requested a review from jorgensd September 15, 2026 09:48
@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from f687380 to 643744a Compare September 15, 2026 13:03
@jhale
jhale marked this pull request as ready for review September 15, 2026 13:05
@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from 643744a to 8570bb6 Compare September 15, 2026 13:12
@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from 8570bb6 to a1a843c Compare September 15, 2026 13:50
@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from a1a843c to 6c4e317 Compare September 15, 2026 13:59
@jorgensd

Copy link
Copy Markdown
Member

I'm a bit worried about this one, as it will be very hard for downstream users to catch (even if it is in the release notes).

@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from 6c4e317 to dc560fa Compare September 15, 2026 18:42
@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from dc560fa to 1f14993 Compare September 16, 2026 08:16
@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from 1f14993 to 8dcab6b Compare September 16, 2026 15:19
@jhale

jhale commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

I'm a bit worried about this one, as it will be very hard for downstream users to catch (even if it is in the release notes).

There is a 'tiny struct trick' in C++ which we can use for one or two rounds of releases to force users to make the switch through the typing system.

@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from 8dcab6b to e1bed85 Compare September 17, 2026 06:27
@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch 2 times, most recently from 1592e56 to 04a03f5 Compare September 17, 2026 16:39
Base automatically changed from jhale/mesh-review-wp3-boundary-validation to main September 18, 2026 07:51
create_submesh returns (submesh, entity_map, vertex_map, geom_map), but
transfer_meshtags_to_submesh took (tags, submesh, vertex_map, cell_map)
- vertex/cell swapped relative to how callers just received them, which
was the natural mistake the F12-1 validation now catches. Reorders to
(tags, submesh, cell_map, vertex_map) so create_submesh's result can be
unpacked and passed straight through. Breaking change, no deprecation
path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jhale
jhale force-pushed the jhale/mesh-review-transfer-meshtags-arg-order branch from 04a03f5 to a096cd2 Compare September 18, 2026 07:51
@jhale

jhale commented Sep 18, 2026

Copy link
Copy Markdown
Member Author

@jorgensd I have another PR which types this, will submit separately.

@jhale
jhale added this pull request to the merge queue Sep 18, 2026
Merged via the queue into main with commit 5efcb4a Sep 18, 2026
22 checks passed
@jhale
jhale deleted the jhale/mesh-review-transfer-meshtags-arg-order branch September 18, 2026 09:40
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.

4 participants