updated checkpoint_xdmf method save discontinuous fields properly #328
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the checkpoint_xdmf method to correctly handle and save discontinuous (cell-based) fields alongside continuous (vertex-based) fields by:
- Introducing a helper to read the size of cell-based fields from HDF5
- Adjusting the mesh filename pattern to include a leading dot before
mesh - Dynamically choosing the XDMF
Center,Dimensions, and HDF5 path based on field continuity
src/underworld3/discretisation.py
Outdated
| def get_cell_field_size(h5_filename): | ||
| with h5py.File(h5_filename, 'r') as f: | ||
| size = f[f'cell_fields/{var.clean_name}_{var.clean_name}'].shape[0] | ||
| return size | ||
|
|
||
| attributes = "" | ||
| for var in meshVars: | ||
| var_filename = filename + f"mesh.{var.clean_name}.{index:05}.h5" | ||
| var_filename = filename + f".mesh.{var.clean_name}.{index:05}.h5" |
There was a problem hiding this comment.
The helper function closes over var.clean_name from the outer scope, which may not refer to the intended variable when called. Consider passing the field name as an explicit parameter or defining this helper within the loop to avoid capturing the wrong var.
Copilot uses AI. Check for mistakes.
src/underworld3/discretisation.py
Outdated
| def get_cell_field_size(h5_filename): | ||
| with h5py.File(h5_filename, 'r') as f: | ||
| size = f[f'cell_fields/{var.clean_name}_{var.clean_name}'].shape[0] | ||
| return size | ||
|
|
||
| attributes = "" | ||
| for var in meshVars: | ||
| var_filename = filename + f"mesh.{var.clean_name}.{index:05}.h5" | ||
| var_filename = filename + f".mesh.{var.clean_name}.{index:05}.h5" |
There was a problem hiding this comment.
[nitpick] Defining get_cell_field_size inside checkpoint_xdmf causes it to be redefined on every call. For clarity and efficiency, move this helper to the module level.
Copilot uses AI. Check for mistakes.
|
|
||
| attributes = "" | ||
| for var in meshVars: | ||
| var_filename = filename + f"mesh.{var.clean_name}.{index:05}.h5" | ||
| var_filename = filename + f".mesh.{var.clean_name}.{index:05}.h5" |
There was a problem hiding this comment.
[nitpick] Building filepaths via string concatenation can lead to errors (e.g. extra dots). Consider using os.path.join or at least a single f-string like f"{filename}.mesh.{var.clean_name}.{index:05}.h5" to improve readability and correctness.
| var_filename = filename + f".mesh.{var.clean_name}.{index:05}.h5" | |
| var_filename = f"{filename}.mesh.{var.clean_name}.{index:05}.h5" |
Copilot uses AI. Check for mistakes.
src/underworld3/discretisation.py
Outdated
|
|
||
| def get_cell_field_size(h5_filename): | ||
| with h5py.File(h5_filename, 'r') as f: | ||
| size = f[f'cell_fields/{var.clean_name}_{var.clean_name}'].shape[0] |
There was a problem hiding this comment.
var is no longer in the scope of this function. This is not good.
Consider passing it in as an argument?
Created new branch to avoid conflicts
This is same as #327
@julesghub I renamed the def and pulled out of the for loop as you suggested