-
Notifications
You must be signed in to change notification settings - Fork 56
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
Multi-DVGeo capability in MPhys #207
Conversation
This reverts commit 15f0162.
Codecov Report
@@ Coverage Diff @@
## main #207 +/- ##
==========================================
- Coverage 64.76% 64.65% -0.12%
==========================================
Files 47 47
Lines 12018 12040 +22
==========================================
Hits 7784 7784
- Misses 4234 4256 +22
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
The changes are good with me, but let's have the actual users review this PR. @bernardopacini @lamkina @ArshSaja Maybe one simplifying pattern could be to always initialize the dvgeos as if you have multiple, but if dvgeo info is not passed, you can just populate that with the single dvgeo's information. Later on in the code, instead of looping over the keys, you can just take the first entry etc. But the way its currently coded is easier to understand; just more lines of code. Either way is fine with me. |
I implemented a version of this and tested it out, let me know what you think @anilyil |
My derivatives are fine. The only thing is that if I need to get some values from the DVGeo in my runscript, I would just use |
I think if you want to call a function that isn't in the MPhys wrapper like |
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.
I tested this with my aeroprop case and everything looks good. LGTM.
Purpose
This adds the ability to add multiple DVGeo objects to one MPhys geometry component when setting up the
OM_DVGEOCOMP
while preserving backward compatibility for the more common single DVGeo case.DVGeometry classes were also modified to standardize
name
andtype
attributes to enable tracking of multiple DVGeos.Expected time until merged
1 week
Type of change
Testing
In addition to the MACH pyGeo tests passing, I have some test scripts for single-DVGeo and multi-DVGeo MPhys in place of actual MPhys tests.
Checklist
flake8
andblack
to make sure the Python code adheres to PEP-8 and is consistently formattedfprettify
or C/C++ code withclang-format
as applicable