Fix skin result extraction based on list of elements #532
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #532 +/- ##
==========================================
- Coverage 83.82% 83.45% -0.37%
==========================================
Files 47 47
Lines 5068 5120 +52
==========================================
+ Hits 4248 4273 +25
- Misses 820 847 +27 |
| ) | ||
| assert len(result.columns.set_ids) == 1 | ||
| if SERVERS_VERSION_GREATER_THAN_OR_EQUAL_TO_7_1: | ||
| assert len(result.index.mesh_index) == 36 |
There was a problem hiding this comment.
how confident are we all these unit tests changes?
There was a problem hiding this comment.
did you plot all of these to make sure they were good?
There was a problem hiding this comment.
@cbellot000 Yes, I debugged most of them locally to check the actual number of elements involved and whether the new reference is correct this time. This is why I switched to NamedSelections for most of them.
There was a problem hiding this comment.
I think we can now duplicate all these tests once with reduce_mesh=True, where we should have the same results as before, right?
There was a problem hiding this comment.
@cbellot000 I started making specific tests for when we use reduce_mesh, but I do not think we need to duplicate all of the already existing ones for each result.
| is computed over list of elements (not supported for cyclic symmetry). Getting the | ||
| skin on more than one result (several time freq sets, split data...) is only | ||
| supported starting with Ansys 2023R2. | ||
| reduce_mesh: |
There was a problem hiding this comment.
what about the external layer?
I'm not sure about the name "reduce" any other ideas, @rlagha @FedericoNegri?
There was a problem hiding this comment.
@cbellot000 you're right, I'll check what it does in combination with external_layer. As for the name reduce_mesh, it is definitely just a proposal. I thought about it though, and to me this is one of the most explicit ways to describe what it does.
| supported starting with Ansys 2023R2. | ||
| Select the skin (creates new 2D elements connecting the external nodes) | ||
| of the mesh for plotting and data extraction. If a list is passed, the skin | ||
| is computed over list of elements (not supported for cyclic symmetry). Getting the |
There was a problem hiding this comment.
isn't this description true only for reduce_mesh==True?
There was a problem hiding this comment.
@cbellot000 do you mean the fact that having skin=[1, 2, 3] will map results onto the skin elements of elements 1, 2, and 3, based on the skin for the initial mesh?
If reduce_mesh=False, you will get results on "skin elements corresponding to elements 1, 2, and 3 based on the skin generated from the initial mesh".
If reduce_mesh=True, as described in the docstring for it, you will get results on the "all skin elements of the skin generated from the mesh composed of elements 1, 2, and 3 only".
JennaPaikowsky
left a comment
JennaPaikowsky
left a comment
There was a problem hiding this comment.
Just a few minor changes
Co-authored-by: JennaPaikowsky <[email protected]>
… fix/skin_result_extraction
Closes #423
Fixes the initial issue while adding a new request argument for mesh reduction
reduce_meshto handle the behavior expected for remote post-processing: