Add the possibility to change the position of the scaling for the chord design variable #410

lucaeros · 2023-05-09T18:36:40Z

Purpose

Scaling position at quarter chord :

Scaling position at trailing edge :

Expected time until merged

Type of change

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (non-backwards-compatible fix or feature)
Code style update (formatting, renaming)
Refactoring (no functional changes, no API changes)
Documentation update
Maintenance update
Other (please describe)

Testing

Checklist

I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
I have run unit and regression tests which pass locally with my changes
I have added new tests that prove my fix is effective or that my feature works
I have added necessary documentation

codecov · 2023-05-09T18:59:46Z

Codecov Report

❗ No coverage uploaded for pull request base (main@2567ea9). Click here to learn what that means.
The diff coverage is 94.44%.

@@           Coverage Diff           @@
##             main     #410   +/-   ##
=======================================
  Coverage        ?   94.51%           
=======================================
  Files           ?      103           
  Lines           ?     6420           
  Branches        ?        0           
=======================================
  Hits            ?     6068           
  Misses          ?      352           
  Partials        ?        0

Impacted Files	Coverage Δ
openaerostruct/geometry/geometry_mesh.py	`98.68% <85.71%> (ø)`
openaerostruct/__init__.py	`100.00% <100.00%> (ø)`
...rostruct/geometry/geometry_mesh_transformations.py	`99.66% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

kanekosh · 2023-05-10T01:13:16Z

Thanks Lucas! Could you add a quick documentation about this new feature in the geometry manipulation and surface dict entries pages?

eytanadler

Thanks for the contributions! In addition to the code comments I added, please also do the following:

Add this new feature to the rectangular wing chord optimization example here.
Add a "chord_scaling_pos" entry to the surface dictionary in this test to test that your change works.
Add another test for ScaleX in this file that checks the partials and confirms that the chord values have changed as you expect.

openaerostruct/geometry/geometry_mesh_transformations.py

eytanadler · 2023-05-10T17:33:36Z

openaerostruct/geometry/geometry_mesh.py

@@ -79,10 +79,15 @@ def setup(self):
        val = np.ones(ny)
        if "chord_cp" in surface:
            promotes = ["chord"]
+            if "chord_scaling_pos" in surface:
+                gamma = surface["chord_scaling_pos"]


Once you change the option name, also change this variable name. Also can't this if else block be moved outside of the if "chord_cp" in surface block so that you don't need to duplicate the gamma = 0.25 line? If you want to be even nicer, you could throw a warning if "chord_scaling_pos" is specified but there are no chord DVs.

openaerostruct/geometry/geometry_mesh_transformations.py

…chordscaling

eytanadler

Thanks for the updates! Other than the two comments, please add documentation to geometry manipulation as Shugo mentioned about this new feature. It'd be nice to add two new pictures in the same format as the others to show what it does as there are for the existing design variables.

eytanadler · 2023-06-02T23:30:22Z

openaerostruct/geometry/geometry_mesh.py

        else:
+            if "chord_scaling_pos" in surface:
+                print("WARNING: chord_scaling_pos has been specified but no chord design variable available")


Use the built in warnings package as we do here. It's nicer because it allows the user to turn off warnings if desired (among other things)

eytanadler · 2023-06-02T23:31:52Z

openaerostruct/geometry/tests/test_geometry_mesh_transformations.py

+        group = prob.model
+
+        val = self.rng.random(NY)
+        chord_scaling_pos = self.rng.random(1)


Can you pick a value here, maybe 0 or 1, that you can analytically check? Then add an assert after run model that checks that the chord has been modified as you expect

Is this still an open item?

lucaeros · 2023-06-07T02:21:09Z

Thanks for the updates! Other than the two comments, please add documentation to geometry manipulation as Shugo mentioned about this new feature. It'd be nice to add two new pictures in the same format as the others to show what it does as there are for the existing design variables.
I added the feature here :
https://mdolab-openaerostruct.readthedocs-hosted.com/en/latest/user_reference/mesh_surface_dict.html#mesh-dict

eytanadler · 2023-06-19T21:13:26Z

Last thing, this one is easy. Could you update the version in the openaerostruct/__init__.py file to 2.6.2? Then I think this is ready

lucaeros · 2023-06-21T13:56:18Z

Last thing, this one is easy. Could you update the version in the openaerostruct/__init__.py file to 2.6.2? Then I think this is ready

done

…ling

bernardopacini

This looks good to me, but what about the open item @eytanadler mentioned above? I agree that fixing it to a value and checking the result is a good idea.

lucaeros · 2023-06-23T22:20:44Z

This looks good to me, but what about the open item @eytanadler mentioned above? I agree that fixing it to a value and checking the result is a good idea.

A test has been added to check if the chord_scaling_pos is set to 1, the Trailing edge should not move when using chord DV.

lucaeros · 2023-06-23T22:21:40Z

This looks good to me, but what about the open item @eytanadler mentioned above? I agree that fixing it to a value and checking the result is a good idea.

A test has been added to check if the chord_scaling_pos is set to 1, the Trailing edge should not move when using chord DV.

Otherwise, the derivatives are check for any number of chord_scaling_pos

bernardopacini

Thanks for adding the test, that looks good to me.

eytanadler · 2023-06-26T13:58:01Z

One more change, but I can make this one. The new test @lucaeros added should be in its own function, not within an existing test method.

chord scaling position

c7b96dd

lucaeros requested a review from a team as a code owner May 9, 2023 18:36

lucaeros requested review from bernardopacini and eytanadler May 9, 2023 18:36

lucaeros added 4 commits May 9, 2023 15:12

flake8

ed77a1c

flake8 balc

569bfbf

git pb

5036e9a

pb git

1267dbc

eytanadler requested changes May 10, 2023

View reviewed changes

lucaeros added 4 commits June 1, 2023 14:58

Merge branch 'main' of https://github.com/mdolab/OpenAeroStruct into …

1a12c19

…chordscaling

add test chord scaling

2c65a1d

change documentation

44d3409

corrected test

d0e2cd3

lucaeros requested a review from eytanadler June 2, 2023 21:30

eytanadler requested changes Jun 2, 2023

View reviewed changes

lucaeros added 2 commits June 7, 2023 04:17

add warnings and check chord scaling

f84553d

stack level

6cf93d1

lucaeros requested a review from eytanadler June 7, 2023 02:21

change version init

0f1f2d3

Merge branch 'main' of github.com:mdolab/OpenAeroStruct into chordsca…

efc35f9

…ling

eytanadler previously approved these changes Jun 21, 2023

View reviewed changes

bernardopacini approved these changes Jun 21, 2023

View reviewed changes

lucaeros requested a review from bernardopacini June 23, 2023 22:20

bernardopacini previously approved these changes Jun 26, 2023

View reviewed changes

Separate chord_scaling_pos test into two separate tests

7a6191c

eytanadler dismissed stale reviews from bernardopacini and themself via 7a6191c June 26, 2023 15:24

eytanadler approved these changes Jun 26, 2023

View reviewed changes

eytanadler merged commit af05b30 into mdolab:main Jun 26, 2023
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the possibility to change the position of the scaling for the chord design variable #410

Add the possibility to change the position of the scaling for the chord design variable #410

lucaeros commented May 9, 2023

codecov bot commented May 9, 2023 •

edited

kanekosh commented May 10, 2023

eytanadler left a comment

eytanadler May 10, 2023

eytanadler left a comment

eytanadler Jun 2, 2023

eytanadler Jun 2, 2023

bernardopacini Jun 21, 2023

lucaeros commented Jun 7, 2023

eytanadler commented Jun 19, 2023 •

edited

lucaeros commented Jun 21, 2023

bernardopacini left a comment

lucaeros commented Jun 23, 2023

lucaeros commented Jun 23, 2023

bernardopacini left a comment

eytanadler commented Jun 26, 2023

Add the possibility to change the position of the scaling for the chord design variable #410

Add the possibility to change the position of the scaling for the chord design variable #410

Conversation

lucaeros commented May 9, 2023

Purpose

Expected time until merged

Type of change

Testing

Checklist

codecov bot commented May 9, 2023 • edited

Codecov Report

kanekosh commented May 10, 2023

eytanadler left a comment

Choose a reason for hiding this comment

eytanadler May 10, 2023

Choose a reason for hiding this comment

eytanadler left a comment

Choose a reason for hiding this comment

eytanadler Jun 2, 2023

Choose a reason for hiding this comment

eytanadler Jun 2, 2023

Choose a reason for hiding this comment

bernardopacini Jun 21, 2023

Choose a reason for hiding this comment

lucaeros commented Jun 7, 2023

eytanadler commented Jun 19, 2023 • edited

lucaeros commented Jun 21, 2023

bernardopacini left a comment

Choose a reason for hiding this comment

lucaeros commented Jun 23, 2023

lucaeros commented Jun 23, 2023

bernardopacini left a comment

Choose a reason for hiding this comment

eytanadler commented Jun 26, 2023

codecov bot commented May 9, 2023 •

edited

eytanadler commented Jun 19, 2023 •

edited