-
Notifications
You must be signed in to change notification settings - Fork 55
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
fixed span derivative issues #143
Conversation
Codecov Report
@@ Coverage Diff @@
## main #143 +/- ##
===========================================
- Coverage 62.77% 51.91% -10.86%
===========================================
Files 46 46
Lines 11263 11266 +3
===========================================
- Hits 7070 5849 -1221
- Misses 4193 5417 +1224
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
refFFDCoef = copy.copy(self.origFFDCoef.astype("D")) | ||
refCoef = copy.copy(self.coef0.astype("D")) | ||
else: | ||
refFFDCoef = copy.copy(self.FFD.coef) |
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.
Why does the reset happen only if the FFD is not a child?
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.
If the FFD is child then it's nodes will be moved by the parent
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.
Without this the test_block test were failing
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.
does that work even if I define the span variable directly on the child?
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'm not sure.
Probably not
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 have added a test using a span variable on a child FFD.
It looks like it worked
@hajdik This is ready for review again |
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.
Thanks for adding the extra test!
Purpose
This PR addresses this issues and issues like it.
Simple design variable functions like this were producing the wrong derivative values because the coefficients used the second time (in the CS used to compute the Jacobain) had already been scaled once.
The fix was simply to reset the FFD coefficient values before the CS loop used to calculate the Jacobian.
Expected time until merged
1 week
Type of change
Testing
See the new test
Checklist
flake8
andblack
to make sure the code adheres to PEP-8 and is consistently formatted