Skip to content
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

Associate the mesh to the Nodes.coordinates field #1473

Merged
merged 5 commits into from
Apr 10, 2024
Merged

Associate the mesh to the Nodes.coordinates field #1473

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c1e4722
Associate the mesh to the returned Nodes.coordinates field for plotti…
PProfizi Mar 18, 2024
8ef3cfe
Add test
PProfizi Mar 20, 2024
649143b
Merge branch 'master' into fix/associate_mesh_to_coordinates_field
PProfizi Mar 20, 2024
77618ba
Merge branch 'master' into fix/associate_mesh_to_coordinates_field
PProfizi Apr 2, 2024
2d9917e
Merge branch 'master' into fix/associate_mesh_to_coordinates_field
PProfizi Apr 9, 2024
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
4 changes: 3 additions & 1 deletion src/ansys/dpf/core/nodes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,9 @@ def coordinates_field(self):
DPFArray([0.015, 0.045, 0.03 ]...

"""
return self._get_coordinates_field()
cf = self._get_coordinates_field()
cf.meshed_region = self._mesh
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this create a sort of circular dependency? The mesh has a field inside (the coordinate field), that has the mesh as its support, that in turn has a coordinate field.... and so on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rafacanton Notice that here I only do that when returning a "standalone" Field. Its associated mesh will indeed have a coordinates PropertyField, but that one will not have an associated mesh until we explicitly request it on its own

Copy link
Contributor

@rafacanton rafacanton Mar 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@PProfizi Yes, I understand that this association will not happen until this method is called. I cannot imagine of any potential downside that this can have down the stream (after having created the circ dep). @cbellot000 can you imagine of any? For example, what if you then add a new node to the mesh? This pushes back an id and 3 doubles in the coord field. Is there any problem in doing this with a field that has as a support the same mesh where this field is stored?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @rafacanton , Hi @cbellot000 ,
If we cannot think of a way this may break something, we might as well merge it.
It is mandatory for several purposes when handling a PropertyField.

return cf

@coordinates_field.setter
@version_requires("3.0")
Expand Down
1 change: 1 addition & 0 deletions tests/test_meshregion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ def test_get_coordinates_field_meshedregion(simple_bar_model):

coordinates = mesh.property_field(dpf.core.common.nodal_properties.coordinates)
assert np.allclose(coordinates.data, field_coordinates.data)
assert np.all(coordinates.meshed_region.nodes.scoping.ids == mesh.nodes.scoping.ids)


@conftest.raises_for_servers_version_under("3.0")
Expand Down
Loading