-
Notifications
You must be signed in to change notification settings - Fork 142
feat: improving performance of save_selection #3697
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
Conversation
|
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
…ithub.com/ansys/pymapdl into feat/improving-save_selection-performance
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3697 +/- ##
==========================================
- Coverage 87.14% 87.09% -0.06%
==========================================
Files 187 187
Lines 14697 14709 +12
==========================================
+ Hits 12808 12811 +3
- Misses 1889 1898 +9 |
|
@pyansys-ci-bot LGTM. |
pyansys-ci-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

Description
As the title. By using components instead of retrieving all entity ids, we speed up the process.
Previous implementation used
XSELfunctions to set the retrieved ids of the entities. While this was OK, it incurs in performance penalization because it has to write a file (sometimes quite big), transfer it and run it usingnon_interactive. Because of the amount ofXSEL,A,...lines, this might posed a problem with the infamous gRPC instability (#3313).I think the thorough testing should be sufficient to ensure we do not hit issues like #3372 or #2452
However can @mikerife have a look at the
test_save_selection_2code and see if I am missing something?Benchmark
Testing code is
test_save_selection_2.Issue linked
Close #3695
Checklist
draftif it is not ready to be reviewed yet.feat: adding new MAPDL command)