fix(fem): resolve an element surface to the face it names, not every node - #375
Merged
Merged
Conversation
…node `surface_nodes` returned every node of every element in an element-based surface, ignoring which face the surface actually named. A C3D10 with one triangular face on a plate end contributed all ten of its nodes, dragging in interior nodes and nodes on adjoining faces. On a 463k-element Abaqus-derived model the three coupling regions came back roughly twice their true size -- shell edge 1473 against 742, solid plate end face 14880 against 7291, lip underside 3286 against 1619 -- so the Sesam writer tied about twice as many nodes rigidly through BLDEP as the model intended, silently stiffening the joint. With this change all three resolve to exactly the intended node sets, compared by node id and not merely by count. The information was already there: an Abaqus deck names the face (`_SURF_S3, S3` for a solid, `E1..E4` for a shell edge) and the reader already stored it as `el_face_index`. Only the resolver ignored it. Adds Abaqus face-to-node maps with mid-side nodes for TETRA/TETRA10, HEX8/HEX20 and WEDGE/WEDGE15, and shell edge maps for TRI/TRI6/QUAD/QUAD8. PYRAMID5, PYRAMID13 and HEX27 are deliberately absent -- Abaqus has no pyramid continuum element and no published C3D27 face numbering to copy -- and a surface naming a face on one of those now raises instead of silently returning every node. Also fixes `get_surface_from_nodes_on_solid_elements`, which took an index into the *visualisation* face table and wrote it as an Abaqus face number. The two agree for tetrahedra and hexahedra by coincidence, but the viz table triangulates a wedge's three quad faces, so it holds six entries for a five-faced element: a wedge could be labelled with the wrong face, or with an "S6" no wedge has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nej3bWxJevs9GkH6tg1WKv
PR Review👋 I checked your PR and found no issues. Thanks!
|
🚀 Profiling Results (Top 20 most expensive calls)
|
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
2 participants
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.
surface_nodesreturned every node of every element in an element-based surface, ignoring which face the surface actually named. A C3D10 with one triangular face on a plate end contributed all ten of its nodes, dragging in interior nodes and nodes on adjoining faces.On a 463k-element Abaqus-derived model the three coupling regions came back at roughly twice their true size:
The Sesam writer then tied about twice as many nodes rigidly through BLDEP as the model intended, silently stiffening the joint. With this change all three resolve to exactly the intended node sets — compared by node id, not merely by count.
The information was already in the deck and in the reader. An Abaqus element surface names its faces (
_LIP_UNDERSIDE_S3, S3for a solid,E1..E4for a shell edge) andread/reader.pyalready stored them asel_face_index. Only the resolver ignored them. One finding worth flagging for review: for a multi-row surface the reader leavesfem_setandel_face_indexasNoneand keeps each row's side label inid_refs[i][1], so that is where the side has to be read from.What is added
Abaqus face-to-node maps including mid-side nodes for TETRA/TETRA10, HEX8/HEX20 and WEDGE/WEDGE15, and shell edge maps for TRI/TRI6/QUAD/QUAD8.
SPOS/SNEGname a shell face — the whole element — and are handled explicitly rather than incidentally.PYRAMID5, PYRAMID13 and HEX27 are deliberately absent: Abaqus has no pyramid continuum element and no published C3D27 face numbering to copy, so there is no authority to follow. A surface naming a face on one of those raises, naming the type and the side, rather than falling back to "every node" — which is the bug this fixes.
A second defect, same area
get_surface_from_nodes_on_solid_elementstook an index into the visualisation face table and wrote it as an Abaqus face number. The two agree for tetrahedra and hexahedra by coincidence, but the viz table triangulates a wedge's three quad faces, so it holds six entries for a five-faced element: a wedge could be labelled with the wrong face, or with anS6that no wedge has. It now matches against the Abaqus table, on corner nodes, so the number written is one the element actually has.elem_has_parallel_facestays for the shell path, which uses it only as a predicate and discards the index.Validation
tests/core/femon current main. The 5 failures in that directory are pre-existing:import freetyperaisesRuntimeError: Freetype library not foundin this environment, in image-rendering tests unrelated to this change.Independent of #374, though both came out of the same Sesam export work.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Nej3bWxJevs9GkH6tg1WKv