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.

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

Conversation

PProfizi
Copy link
Contributor

This made the coordinates field unavailable for plotting and further handling by Post

…ng and further handling by Post

Signed-off-by: paul.profizi <paul.profizi@ansys.com>
@PProfizi PProfizi added the bug Something isn't working label Mar 18, 2024
@PProfizi PProfizi self-assigned this Mar 18, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.81%. Comparing base (a9d68bb) to head (77618ba).
Report is 2 commits behind head on master.

❗ Current head 77618ba differs from pull request most recent head 2d9917e. Consider uploading reports for the commit 2d9917e to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1473   +/-   ##
=======================================
  Coverage   86.81%   86.81%           
=======================================
  Files          82       82           
  Lines        9631     9633    +2     
=======================================
+ Hits         8361     8363    +2     
  Misses       1270     1270           

Signed-off-by: paul.profizi <paul.profizi@ansys.com>
@wiz-inc-572fc38784
Copy link

wiz-inc-572fc38784 bot commented Mar 20, 2024

Wiz Scan Summary

IaC Misconfigurations 0C 0H 0M 0L 0I
Vulnerabilities 0C 0H 0M 0L 0I
Sensitive Data 0C 0H 0M 0L 0I
Total 0C 0H 0M 0L 0I
Secrets 0🔑

@@ -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

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.

@PProfizi PProfizi merged commit 73640db into master Apr 10, 2024
37 checks passed
@PProfizi PProfizi deleted the fix/associate_mesh_to_coordinates_field branch April 10, 2024 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants