-
Notifications
You must be signed in to change notification settings - Fork 112
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
Fixing symmetry for right wings #381
Conversation
Codecov Report
@@ Coverage Diff @@
## main #381 +/- ##
==========================================
+ Coverage 96.61% 96.63% +0.01%
==========================================
Files 93 93
Lines 5912 5937 +25
==========================================
+ Hits 5712 5737 +25
Misses 200 200
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
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.
Thank you @timryanb! I added a few comments on the new regression tests.
|
||
prob.run_model() | ||
|
||
assert_near_equal(prob["aero_point_0.wing_perf.CD"][0], 0.03487336411850356, 1e-6) |
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.
Instead of the hardcoded values, would you be able to do the following?
- Run the original left-wing case (duplicate of
test_simple_rect_aero.py
) - Run the right-wing case
- Assert_near_equal between the left-wing results and right-wing results
I saw that the hardcoded values here are identical to test_simple_rect_aero.py
, so my suggestion is equivalent to the current assert_equal. But I think it'd be safer to compare the left-wing and right-wing directly, in case we need to change the reference values in the future.
This will double the test time, but it should be fine for this small mesh case.
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.
Should be addressed now
|
||
prob.run_driver() | ||
# docs checkpoint 1 | ||
assert_near_equal(prob["aero_point_0.wing_perf.CD"][0], 0.033389699871650073, 1e-6) |
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.
Same as I commented in test_simple_rect_right_aero.py
: instead of the hardcoded values, would you be able to do the following?
- Run the original left-wing case (duplicate of test_aero_ground_effect.py)
- Run the right-wing case
- Assert_near_equal between the left-wing results and right-wing results
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.
Should be addressed now
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.
Thank you @timryanb! I'll merge this PR after fixing black
in a separate PR - they recently broke a thrid-party dependency and we need to version up black
to fix that.
Maybe we also want to bump the version and make a new release? |
Purpose
This PR addresses #244. Symmetric wings can now be used with the aerodynamics solver if they lie in the right side of the symmetry plane. Prior to this, OAS only gave correct results for left wings.
Expected time until merged
a week
Type of change
Testing
This fix can be tested by taking a standard left wing model and flipping the y nodes a shown below:
The new flipped surface can be passed to the aerodynamic component and should give a consistent solution to a left wing or full wing model.
Checklist
flake8
andblack
to make sure the code adheres to PEP-8 and is consistently formatted