-
Notifications
You must be signed in to change notification settings - Fork 104
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
Eval funcs docs #240
Eval funcs docs #240
Conversation
Codecov Report
@@ Coverage Diff @@
## main #240 +/- ##
=======================================
Coverage 42.01% 42.01%
=======================================
Files 13 13
Lines 3999 3999
=======================================
Hits 1680 1680
Misses 2319 2319
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
db9712c
to
8f398bc
Compare
@ArshSaja Please document the remaining functions. That would be every function after To others, feel free to check the changes I have made so far so we can iterate. I would appreciate some help with all the aircraft stability derivatives. @lamkina feel free to add your stuff to this too. Thanks! |
In #231, we ran into an issue with the |
Yep, this is exactly the place to put that documentation. It is not currently in this PR so I will add it to my TODO to specify that the compute cavitation flag results in extra computations in the cost function description. |
I will work on this during the break. |
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 fixing all the formatting stuff @gang525! I think this is ready to go.
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 am hitting request changes; I want to review the explanations. Will try to do it later this week. Please finish reviewing other discussions in the meanwhile.
Thanks @gang525 for all changes! I will try to fix it if anything new comes up |
The index TOC depth is now changed from 2 to 1. I corrected some of the other items, but @ArshSaja do you know what the flowpower cost functional is? This PR is getting close to ready. EDIT: this is ready for review |
I have pushed my changes to the descriptions. Someone else should review. @sseraj can you add brief descriptions to the time spectral functions and remove the "TODO"s ? |
Yes, I can do that. |
I'll wait for the tests just to be sure and merge this. |
Purpose
Address #206
Also, all the time spectral aircraft stability derivatives are allegedly broken so the documentation of cost functions says so Issue #295 .
Fixes some typos in source code as well.
Do not merge until these are checked
Expected time until merged
Would really appreciate reviews before all the senior students graduate so within the next semester.
EDIT: Before September 2023
Type of change
Testing
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