B3 fix - #46
Conversation
|
|
||
| """ | ||
| if solver == "vabs" or solver == "anbax": | ||
| if solver == "vabs" or solver == "b3_secfem": |
There was a problem hiding this comment.
Flagging to check that the solver input should be one of these and not anba. Do we ever check that the user gives a valid input for sovler?
| requires-python = ">=3.9" | ||
| license = {text = "MIT"} | ||
| keywords = ["Helicopter","Rotor Blade","OpenMdao","Preprocessor","SONATA","VABS","ANBA"] | ||
| keywords = ["Helicopter","Rotor Blade","OpenMMDAO","Preprocessor","SONATA","VABS","b3_secfem", "b3_secfem"] |
There was a problem hiding this comment.
Why is b3_secfem here twice?
| -3.3776077839723290e-01 -8.1393723459371863e+00 1.8617318932166703e-14 6.5218805541439177e-17 -8.8429149001247509e-16 2.8183512372423348e+05 | ||
| 1.9665378682881441e+07 6.3056114727279851e+02 0.0000000000000000e+00 0.0000000000000000e+00 0.0000000000000000e+00 -3.4242178407083868e-01 | ||
| 6.3056114727869146e+02 5.6354294666494809e+07 0.0000000000000000e+00 0.0000000000000000e+00 0.0000000000000000e+00 -8.1547402643002442e+00 | ||
| 0.0000000000000000e+00 0.0000000000000000e+00 2.1831356082261625e+08 -6.2101162109742521e-05 5.9039054767654111e+00 0.0000000000000000e+00 |
There was a problem hiding this comment.
Flagging the '-6.2101162109742521e-05 ' to look more closely since it seems like a big change, but also a really small number, so not sure yet if it is an issue.
There was a problem hiding this comment.
The ratios between all the components are still consistent, so this is fine.
|
I don't have any concerns with this PR. A few minor comments above, but also okay with just merging since those aren't major issues introduced here. I just looked through the PR diffs. I would separately want to verify all of the b3_secfem work is fully consistent/all the theory in that code makes sense with SONATA. |
PorterJustin
left a comment
There was a problem hiding this comment.
See comments on PR for potential improvements, but okay with me to merge.
This PR allows to run b3_secfem, commit https://github.com/gbarter/b3_secfem/commit/8d6ac3943a2648ff787f93071f8c820a6219455b
Two key changes:
SONATA/SONATA/anbax/anbax_utl.py
Line 30 in 73281a0
8d6ac39to b3_secfemTests 01, 02, and 07 still required an updated of the reference values, but changes are hopefully acceptably small. See below a comparison for the K and I matrices for the IEA15 reference wind turbine. Note that the M matrix is nearly identical, whereas some (negligibly small?) differences persist in K
Two additional changes:
environment.yamlDiagonal of K, y on log scale

Diagonal of M, y on log scale

First row of K, y NOT on log scale

Second row of K, y NOT on log scale

Third row of K, y NOT on log scale

Fourth row of K, y NOT on log scale

Fifth row of K, y NOT on log scale

Sixth row of K, y NOT on log scale

Off diagonal terms of M, y NOT on log scale

Once b3_secfem is made public, tests should start to pass on GitHub actions. Acknowledgments will also be due to @wr1. Lastly, we should ask co-pilot to overhaul the documentation of SONATA, which is terribly outdated