Mesh representation as internal input file storage by isteinbrecher · Pull Request #633 · beamme-py/beamme

isteinbrecher · 2026-04-27T08:43:46Z

This PR changes the 4C input file, such that all added mesh information (except legacy dat style strings) is stored inside a MeshRepresentation in the input file. For now nothing changes in the behaviour. In the future, this will allow us to chose at dump if the mesh information is written to the input file or stored as a external mesh format.

#594

Copilot

Pull request overview

This PR refactors 4C input generation so that newly added mesh data is stored internally as a MeshRepresentation (instead of directly as legacy text-based sections), while preserving current external behavior by converting back to legacy format during dump().

Changes:

Introduces MeshRepresentation-based mesh storage in InputFile, with legacy dump conversion via dump_mesh_representation_to_input_file_legacy().
Updates mesh/material/function/geometry-set dumping to rely on object-to-ID conversion through FourCIPP type converters.
Adjusts integration/unit tests for the new internal representation and temporarily marks VTK output comparisons as xfail.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/integration/test_integration_four_c_header_functions.py	Validates NOX xml path by re-loading dumped YAML via FourCIPP.
tests/integration/test_integration_four_c.py	Adapts NURBS import test to new mesh handling (mesh representation stored in `InputFile`).
tests/integration/test_integration_core_vtk_writer.py	Marks VTK output tests as xfail pending upcoming VTK output changes.
tests/create_test_models.py	Updates how materials are written into Cubit-generated test models.
tests/conftest_result_comparison.py	Converts `InputFile` into legacy-dumped dict for comparisons without mutating the original.
tests/beamme/four_c/test_beamme_four_c_material.py	Updates expected material dump structure (now relies on object conversion).
src/beamme/space_time/beam_to_space_time.py	Updates space-time node typing and VTK legacy cell type naming for space-time elements.
src/beamme/four_c/model_importer.py	Updates mapping key used for 4C-cell → VTK-cell type translation.
src/beamme/four_c/material.py	Changes material dump format to emit material objects for later conversion to IDs.
src/beamme/four_c/input_file_mappings.py	Renames legacy VTK-cell mapping and adds VTK→4C connectivity remapping for legacy dump.
src/beamme/four_c/input_file_dump_item.py	Major refactor: dumping via mesh representation + legacy conversion function.
src/beamme/four_c/input_file.py	Stores mesh as `mesh_representation` internally and dumps via a legacy conversion on a copy.
src/beamme/four_c/element_solid.py	Restricts valid solid materials for 4C solid elements.
src/beamme/four_c/element_beam.py	Removes bespoke 4C dump hook and shifts checks into `check()`.
src/beamme/core/vtk_writer.py	Updates node set naming logic to avoid reliance on `geometry_set.i_global`.
src/beamme/core/nurbs_patch.py	Adds a generic PyVista cell type for NURBS patches and removes legacy global index fields.
src/beamme/core/node.py	Adds explicit `node_type` class attributes for node variants.
src/beamme/core/mesh_representation.py	Adds helpers (`n_points`, `n_cells`, iterators, offsets) and mesh representation merging.
src/beamme/core/mesh.py	Adds `get_mesh_representation()` to build `MeshRepresentation` + ID mappings without mutating mesh long-term.
src/beamme/core/function.py	Restores `i_global` storage on `Function` objects.
src/beamme/core/element_volume.py	Splits VTK cell typing into PyVista type vs legacy VTK class type.
src/beamme/core/element_beam.py	Sets PyVista cell type and removes duplicated material checking (now in `Element`).
src/beamme/core/element.py	Introduces a shared `check()` and material validation logic.
src/beamme/core/base_mesh_item.py	Removes `i_global` from base mesh items (moved to specific types).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

isteinbrecher · 2026-04-28T06:52:22Z

        coupling_sets=True,
    )
-    assert_results_close(ref_file, vtk_file)
+    # assert_results_close(ref_file, vtk_file)


@davidrudlstorfer with the restructuring, the vtk tests fail because the order of the geometry sets is different now also for the vtk output - however, since I want to change the way we write output to simply use the mesh representation, I decided not to fix the implementation and deactivate the result check of the tests. This does not change anything about the display_pyvista functionality.

davidrudlstorfer

@isteinbrecher that review took a little bit longer based on the really large amount of changes, I started yesterday and was able to finish with the review today :D

Overall this is already looking really amazing, a lot of points are just coding style or formatting. The only thing I do not really like currently is the "manual"/"external" function that needs to be called to convert the mesh representation of an input file (see below).

davidrudlstorfer · 2026-04-27T14:42:25Z

+            element_type_id_to_data: A dictionary that maps the element type id to the data of the element type.
+            geometry_sets_to_i_global: A dictionary that maps geometry sets to their global index in the mesh representation.


would it make sense to unify this "id" and "i", both of them are IDs

YES. That has actually been really bothering me with this PR, the different names we use for global IDs. I think I would prefer gid and try to change the ones where it is possible to be index-0 based. I am not sure about materials, but I think functions can not start with 0, because 0 indicates a constant value in 4C.

I would do this in a follow up PR.

Should we open up an issue for that?

davidrudlstorfer · 2026-04-27T14:43:15Z

+        if len(all_materials) != len(material_to_i_global):
+            raise ValueError("Materials are not unique!")


How can this happen? Can this also happen if I just assign the same material to two different beam sets?

This can happen if the user explicitly adds a material to the mesh.materials list more than once.

In the past this caused an inconsistency in the input file, because we assigned the GIDs to the material object and then we would have assigned a GID to the same material twice.

With the new way we assign material GIDs (material_to_i_global), this can't happen any more, so we actually could remove the error. We could also keep it, as it still indicates a state of the mesh where it likely should not be.

If it is not even able to happen anymore in a meaningful way I would remove it, it's more or less dead code then

davidrudlstorfer · 2026-04-27T14:44:07Z

+            raise ValueError("Nodes are not unique!")
+        points = _np.zeros((n_nodes, 3))
+        point_types = _np.full(n_nodes, -1)
+        control_point_weights = None


Could we maybe add nurbs to this variable name? I am not working a lot with them so it would be easier to differentiate and directly see that we handle nurbs stuff here

This would still be an open point for me

Suggested change

control_point_weights = None

nurbs_control_point_weights = None

davidrudlstorfer · 2026-04-27T15:01:16Z


-    return patch_elements

+def dump_mesh_representation_to_input_file_legacy(


Same as last time, would text based or something similar make sense here? Legacy is not really connected to the text based input file

Is this not the way we call the dat-style strings in an inout file? I know we talked about this recently, but after revisiting this here, I think legacy actually makes more sense (though it might not be 100% technically correct).

So legacy are only the parts in 4C which are not converted to yaml style input functionality, such as the elements or nodes

But as far as I understand the mesh representation also holds stuff like functions or materials. So I think dump_mesh_representation_to_text_input_file or even dump_mesh_representation_to_yaml is easier to understand

davidrudlstorfer · 2026-04-27T15:01:50Z

Probably it would also make sense to rename this file, we are no longer dumping items but whole meshes + conversions + ...

Also this still applies

davidrudlstorfer · 2026-04-27T15:02:51Z

+            # To avoid this, we convert the rotations to BeamMe internal rotations
+            # and extract the rotation vectors again, ensuring they are in the
+            # correct range.
+            # This is super slow, but for now keep it, as a mesh based output is to
+            # be preferred when performance is of importance.


Why do we not need this with the mesh based input? I guess we also want definite rotations?

You are right, this might lead to non-unique rotation vector in an external mesh file. However, in 4C this does not matter, since we convert it to quaternions anyways.

The question that remains is how we handle this in testing. There I would maybe simply add a conversion in the future that compares that array in a $SO(3)$ metric, where non-unique rotation vectors won't make a difference. (At least that is the plan, lets see if that works as expected)

Should we also add this as a todo/issue?

davidrudlstorfer · 2026-04-28T07:16:21Z

+        fourc_input_file = obj.fourc_input.copy()
+        dump_mesh_representation_to_input_file_legacy(
+            fourc_input_file, obj.mesh_representation, obj.element_type_id_to_data
+        )


i do not really like this approach that we now need, i.e., that we need an external function call to convert the mesh representation to a "dumpable" input file

couldn't we just use some boolean parameter that does that automatically? For example if i call input_file.sections I would probably automatically expect the input file in the full text based format?

Also in the dump function I would probably just add a parameter like "dump_type" which can either be "text_based" or "binary" and depending on that everything is done automatically

It's a little bit intuitive that you now need external functions imported from another file to convert the mesh_representation

Especially because the argument is only consisting of input files if I see it correctly?

fourc_input file is an input file copied (why actually the copy)?

and then all arguments of the dump_mesh_representation_to_input_file_legacy are input files again.

In my mind this should simply be

input_file.convert_mesh_repr_to_text_based_input()

Without any necessary arguments or whatsoever

Also related to the previous comments. Up until now we could do the comparison by simply looking at input_file.sections. That worked, because when we added a mesh to the input file we dumped all mesh information to input_file.sections. So at this point all the information was already in the input_file.sections.

Now, we have input_file.sections and input_file.mesh_representation and want to compare this to input files on disk. So we have to add the mesh information to the input_file.sections here somehow so we can compare it.

What we could do is add a method

def InputFile(): ... def get_copy_with_legacy_mesh_sections(self) -> FourCInput: fourc_input_file_with_mesh = self.fourc_input.copy() dump_mesh_representation_to_input_file_legacy( fourc_input_file_with_mesh, self.mesh_representation, self.element_type_id_to_data ) return fourc_input_file_with_mesh

and then call this method here to get the input file sections we need for testing comparisons.

I would prefer not getting copies of data because this get's imho dirty quickly in python. I could still imagine some third data type in the input file

input_file.sections_from_mesh_representation which we fill with input_file.mesh_representation_to_sections. During the dump we can simply merge the two in a temporary fourcipp input file and write it to disk

isteinbrecher · 2026-04-29T05:46:58Z

@davidrudlstorfer thanks for the thorough review! Feel free to close resolved discussions. And sorry for a maybe too short description for such a long PR.

davidrudlstorfer

I hope I have answered everything, please just ping me if I have forgotten any comment :)

This already is pretty close, I would love to resolve the comment formatting somehow and the copies of the input file. Maybe we find a good solution on both :)

davidrudlstorfer · 2026-04-30T11:38:20Z

+            element_type_id_to_data: A dictionary that maps the element type id to the data of the element type.
+            geometry_sets_to_i_global: A dictionary that maps geometry sets to their global index in the mesh representation.


Should we open up an issue for that?

davidrudlstorfer · 2026-04-30T11:39:25Z

+        if len(all_materials) != len(material_to_i_global):
+            raise ValueError("Materials are not unique!")


If it is not even able to happen anymore in a meaningful way I would remove it, it's more or less dead code then

davidrudlstorfer · 2026-04-30T11:39:51Z

+            raise ValueError("Nodes are not unique!")
+        points = _np.zeros((n_nodes, 3))
+        point_types = _np.full(n_nodes, -1)
+        control_point_weights = None


This would still be an open point for me

Suggested change

control_point_weights = None

nurbs_control_point_weights = None

davidrudlstorfer · 2026-04-30T11:43:45Z

+        #   We first have to loop over all elements, so we get the total
+        #   number of elements, as NURBS patches can contain multiple elements.
+        #   This is needed to initialize the numpy arrays with the correct size.
+        i_element = 0


I get your point but I would still aim for a consistent commenting style. I do not really know how these comment play along with our formaters tbh.

When one reads the code from the top to bottom it is clear anyways that these parts fit together.

If a separation is really necessary it would probably make sense to put that stuff into its own function? Then it can be documented nicely and all the comments belong to this one "task"

Another option I would see is to start the first comment with some sort of headline/step

Something like

# Step 1: Extract elements for the mesh representation

or

# First step; extract elements for the mesh representation

and so on for the next steps.

Looking at the function being something like 230 lines long, some inner functions would probably make this even easier to read and differentiate

davidrudlstorfer · 2026-04-30T11:44:56Z

+        element_type_to_id: dict[type, int] = {}
+        element_type_id_to_data: dict[int, dict] = {}
+        cell_connectivity = []
+        cell_types = _np.full(n_elements, -1)
+        cell_element_type_ids = _np.full(n_elements, -1)
+        cell_material_ids = _np.full(n_elements, -1)
+        cell_beamme_element_ids = _np.full(n_elements, -1)


I still think the appending and then converting it to numpy where necessary is nicer. We have this approach of pre-allocating the arrays nowhere in our code base and we should stay consistent. There's also no performance benefit in doing so. In my opinion this only makes the code harder to read.

davidrudlstorfer · 2026-04-30T12:14:50Z

+    #   If there are couplings in the mesh, set the link between the nodes
+    #   and elements, so the couplings can decide which DOFs they couple,
+    #   depending on the type of the connected beam element.


Same as all above

davidrudlstorfer · 2026-04-30T12:16:39Z


-    return patch_elements

+def dump_mesh_representation_to_input_file_legacy(


So legacy are only the parts in 4C which are not converted to yaml style input functionality, such as the elements or nodes

But as far as I understand the mesh representation also holds stuff like functions or materials. So I think dump_mesh_representation_to_text_input_file or even dump_mesh_representation_to_yaml is easier to understand

davidrudlstorfer · 2026-04-30T12:17:03Z

Also this still applies

davidrudlstorfer · 2026-04-30T12:18:22Z

+            # To avoid this, we convert the rotations to BeamMe internal rotations
+            # and extract the rotation vectors again, ensuring they are in the
+            # correct range.
+            # This is super slow, but for now keep it, as a mesh based output is to
+            # be preferred when performance is of importance.


Should we also add this as a todo/issue?

davidrudlstorfer · 2026-04-30T12:21:14Z

+        fourc_input_file = obj.fourc_input.copy()
+        dump_mesh_representation_to_input_file_legacy(
+            fourc_input_file, obj.mesh_representation, obj.element_type_id_to_data
+        )


I would prefer not getting copies of data because this get's imho dirty quickly in python. I could still imagine some third data type in the input file

input_file.sections_from_mesh_representation which we fill with input_file.mesh_representation_to_sections. During the dump we can simply merge the two in a temporary fourcipp input file and write it to disk

Add n_cells and n_points property to mesh representation

5f5a5e3

isteinbrecher requested review from Copilot and davidrudlstorfer April 27, 2026 08:43

isteinbrecher had a problem deploying to cubit_secrets_trusted April 27, 2026 08:43 — with GitHub Actions Failure

Copilot AI reviewed Apr 27, 2026

View reviewed changes

isteinbrecher force-pushed the mesh-representation-as-internal-inpout-file-storage branch from 594cc86 to a9d02f5 Compare April 27, 2026 09:01

isteinbrecher had a problem deploying to cubit_secrets_trusted April 27, 2026 09:01 — with GitHub Actions Failure

isteinbrecher force-pushed the mesh-representation-as-internal-inpout-file-storage branch from a9d02f5 to ac3036c Compare April 27, 2026 09:30

isteinbrecher had a problem deploying to cubit_secrets_trusted April 27, 2026 09:30 — with GitHub Actions Failure

isteinbrecher force-pushed the mesh-representation-as-internal-inpout-file-storage branch from ac3036c to 0f4b4c7 Compare April 27, 2026 09:46

isteinbrecher had a problem deploying to cubit_secrets_trusted April 27, 2026 09:46 — with GitHub Actions Failure

isteinbrecher requested a review from Copilot April 27, 2026 09:46

Copilot started reviewing on behalf of isteinbrecher April 27, 2026 09:46 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread src/beamme/core/element.py Outdated

Comment thread src/beamme/core/vtk_writer.py

Comment thread tests/conftest_result_comparison.py Outdated

Comment thread src/beamme/four_c/input_file_dump_item.py Outdated

Comment thread src/beamme/core/element_volume.py

isteinbrecher force-pushed the mesh-representation-as-internal-inpout-file-storage branch from 0f4b4c7 to 4a4c172 Compare April 27, 2026 10:15

isteinbrecher had a problem deploying to cubit_secrets_trusted April 27, 2026 10:15 — with GitHub Actions Failure

isteinbrecher requested a review from Copilot April 27, 2026 10:15

Copilot started reviewing on behalf of isteinbrecher April 27, 2026 10:16 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

isteinbrecher force-pushed the mesh-representation-as-internal-inpout-file-storage branch from 4a4c172 to 89db7c1 Compare April 27, 2026 10:36

isteinbrecher temporarily deployed to cubit_secrets_trusted April 27, 2026 10:36 — with GitHub Actions Inactive

Use mesh representation as interal data format for 4C input file

b9f7b58

isteinbrecher force-pushed the mesh-representation-as-internal-inpout-file-storage branch from 89db7c1 to 33d1f1a Compare April 27, 2026 10:38

isteinbrecher temporarily deployed to cubit_secrets_trusted April 27, 2026 10:38 — with GitHub Actions Inactive

isteinbrecher requested a review from Copilot April 27, 2026 10:39

Copilot started reviewing on behalf of isteinbrecher April 27, 2026 10:40 View session

isteinbrecher had a problem deploying to cubit_secrets_trusted April 27, 2026 10:45 — with GitHub Actions Failure

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Comment thread tests/conftest_result_comparison.py Outdated

Comment thread src/beamme/four_c/input_file_dump_item.py

isteinbrecher added 2 commits April 27, 2026 16:05

Adapt testing to new input file data storage

9223e50

Fix vtk tests - temporarily

549d20c

isteinbrecher force-pushed the mesh-representation-as-internal-inpout-file-storage branch from 33d1f1a to 549d20c Compare April 27, 2026 14:06

isteinbrecher temporarily deployed to cubit_secrets_trusted April 27, 2026 14:06 — with GitHub Actions Inactive

isteinbrecher had a problem deploying to cubit_secrets_trusted April 27, 2026 14:12 — with GitHub Actions Failure

isteinbrecher commented Apr 28, 2026

View reviewed changes

davidrudlstorfer reviewed Apr 28, 2026

View reviewed changes

isteinbrecher added 5 commits April 28, 2026 17:28

Remove valid materials check

cf845b1

Update comment on elements without material

1f51f04

Update comment on mypy cast

c1cff94

Update FourCIPP comments

47c5934

Add TODOs to vtu tests

576b246

isteinbrecher temporarily deployed to cubit_secrets_trusted April 29, 2026 10:12 — with GitHub Actions Inactive

isteinbrecher had a problem deploying to cubit_secrets_trusted April 29, 2026 10:18 — with GitHub Actions Failure

Move dump item function into caller

ba214c1

isteinbrecher temporarily deployed to cubit_secrets_trusted April 29, 2026 11:27 — with GitHub Actions Inactive

isteinbrecher had a problem deploying to cubit_secrets_trusted April 29, 2026 11:34 — with GitHub Actions Failure

davidrudlstorfer reviewed Apr 30, 2026

View reviewed changes

		element_type_id_to_data: A dictionary that maps the element type id to the data of the element type.
		geometry_sets_to_i_global: A dictionary that maps geometry sets to their global index in the mesh representation.

		if len(all_materials) != len(material_to_i_global):
		raise ValueError("Materials are not unique!")

	control_point_weights = None
	nurbs_control_point_weights = None


		return patch_elements

		def dump_mesh_representation_to_input_file_legacy(

Conversation

isteinbrecher commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidrudlstorfer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

isteinbrecher Apr 29, 2026 •

edited

Loading