-
Notifications
You must be signed in to change notification settings - Fork 525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add VTU export for Unstructured meshes #3290
base: develop
Are you sure you want to change the base?
Add VTU export for Unstructured meshes #3290
Conversation
@shimwell With this PR one should be able to mesh CadQuery shapes, and export legacy VTK, or VTU, without running into module conflicts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rherrero-pf for the contribution! The additional format support will be nice to have. And thank you for updating the VTK imports.
Mostly comments on the new test here.
if export_type == ".vtk": | ||
reader = vtkIOLegacy.vtkGenericDataObjectReader() | ||
reader.SetFileName(str(filename)) | ||
reader.Update() | ||
assert reader.GetOutput().GetCellData().GetArray("mean") | ||
|
||
elif export_type == ".vtk": | ||
reader = vtkIOXML.vtkGenericDataObjectReader() | ||
reader.SetFileName(str(filename)) | ||
reader.Update() | ||
assert reader.GetOutput().GetCellData().GetArray("mean") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should one of these conditions check export_type
against ".vtu"
instead of ".vtk"
?
Since the tests are currently passing, and if the vtkIOXML.vtkGenericDataObjectReader
works for either format , maybe we can unify these blocks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably also verify that the data retrieved matches what was written here.
materials=materials, geometry=geometry, settings=settings, tallies=tallies | ||
) | ||
|
||
with tempfile.TemporaryDirectory() as tmpdir: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the run_in_tmpdir
fixture to this test should do the same thing as this context manager.
def test_umesh(run_in_tmpdir, export_type):
...
statepoint_file = model.run(cwd=tmpdir) | ||
|
||
statepoint = openmc.StatePoint(statepoint_file) | ||
tally: openmc.Tally = statepoint.get_tally(name="test_tally") | ||
umesh_from_sp: openmc.UnstructuredMesh = statepoint.meshes[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically best to open the statepoint files in a context manager to avoid an occupied HDF5 file handle:
statepoint_file = model.run(cwd=tmpdir) | |
statepoint = openmc.StatePoint(statepoint_file) | |
tally: openmc.Tally = statepoint.get_tally(name="test_tally") | |
umesh_from_sp: openmc.UnstructuredMesh = statepoint.meshes[1] | |
with openmc.StatePoint(statepoint_file) as statepoint: | |
tally: openmc.Tally = statepoint.get_tally(name="test_tally") | |
umesh_from_sp: openmc.UnstructuredMesh = statepoint.meshes[1] |
Parameters | ||
---------- | ||
filename : str or pathlib.Path | ||
Name of the VTK file to write | ||
Name of the VTK file to write. If the filename ends in '.vtu' then a VTU |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name of the VTK file to write. If the filename ends in '.vtu' then a VTU | |
Name of the VTK file to write. If the filename ends in '.vtu' then a binary VTU |
Description
Added method to export an unstructured mesh in .vtu format, and removed the dependency on the VTK module, which clashes with CadQuery.
Fixes # (issue)
Fixes issue #3289
Checklist