Skip to content

Meshes are now iterable #295

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 19, 2025
Merged

Meshes are now iterable #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c72f59b
Mesh is now iterable
Mar 14, 2025
90ae934
Fix linting error C420
ddundo Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion goalie/error_estimation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def get_dwr_indicator(F, adjoint_error, test_space=None):
elif isinstance(test_space, WithGeometry):
if len(adjoint_error.keys()) != 1:
raise ValueError("Inconsistent input for 'adjoint_error' and 'test_space'.")
test_space = {key: test_space for key in adjoint_error}
test_space = dict.fromkeys(adjoint_error, test_space)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this. I spotted it in the weekly CI outputs but hadn't got round to it.

elif not isinstance(test_space, dict):
raise TypeError(
"Expected 'test_space' to be a FunctionSpace or dict, not"
Expand Down
4 changes: 1 addition & 3 deletions goalie/function_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,7 @@ def __init__(self, time_partition, meshes):
in time
:arg meshes: the list of meshes used to discretise the problem in space
"""
self._label_dict = {
time_dep: ("error_indicator",) for time_dep in ("steady", "unsteady")
}
self._label_dict = dict.fromkeys(("steady", "unsteady"), ("error_indicator",))
super().__init__(
time_partition,
{
Expand Down
4 changes: 2 additions & 2 deletions goalie/mesh_seq.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def __init__(self, time_partition, initial_meshes, **kwargs):
take various types
"""
self.time_partition = time_partition
self.fields = {field_name: None for field_name in time_partition.field_names}
self.fields = dict.fromkeys(time_partition.field_names)
self.field_types = dict(zip(self.fields, time_partition.field_types))
self.subintervals = time_partition.subintervals
self.num_subintervals = time_partition.num_subintervals
Expand Down Expand Up @@ -166,7 +166,7 @@ def set_meshes(self, meshes):
:class:`firedrake.MeshGeometry`
"""
# TODO #122: Refactor to use the set method
if not isinstance(meshes, Iterable):
if not isinstance(meshes, list):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we allow tuple to cover more possible cases?

Suggested change
if not isinstance(meshes, list):
if not isinstance(meshes, (list, tuple)):

Copy link
Member Author

@ddundo ddundo Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I committed this but I'm not sure we should allow tuples. In adaptor functions in demos we have mesh_seq[i] = adapt(mesh_seq[i], metric), so we need a mutable object to be able to reassign meshes, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and we indeed get an error:

  File "/home/firedrake/goalie/demos/burgers-hessian.py", line 145, in adaptor
    mesh_seq[i] = adapt(mesh_seq[i], metric)
    ~~~~~~~~^^^
  File "/home/firedrake/goalie/goalie/mesh_seq.py", line 130, in __setitem__
    self.meshes[subinterval] = mesh
    ~~~~~~~~~~~^^^^^^^^^^^^^
TypeError: 'tuple' object does not support item assignment

So I reverted this. Am I missing something? Sorry if I am!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I always forget about this. Thanks.

meshes = [Mesh(meshes) for subinterval in self.subintervals]
self.meshes = meshes
dim = np.array([mesh.topological_dimension() for mesh in meshes])
Expand Down