-
Notifications
You must be signed in to change notification settings - Fork 70
WIP: Classes refactor & scipy.ode dense output #104
Conversation
… the internal _ode
… time must be updated manually
Codecov didn't trigger and Travis already submitted the results... Looking into it. |
The results are there https://codecov.io/gh/AeroPython/PyFME/pull/104/commits and both GitHub and TravisCI report all systems operational. @AlexS12 would you please do |
(Edit: commit && push) |
|
Codecov Report
@@ Coverage Diff @@
## master #104 +/- ##
=========================================
+ Coverage 87.25% 92.86% +5.6%
=========================================
Files 28 41 +13
Lines 1914 2185 +271
=========================================
+ Hits 1670 2029 +359
+ Misses 244 156 -88
Continue to review full report at Codecov.
|
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 already had a look to this in full. Comparing the diff makes no sense since this is a full refactor of almost everything. The classes look good, the notebook is awesome and for me this is ready to go :)
I made a las push just to make sure my changes trigger all the CI process correctly. I will write a final message here covering what has been going on and what is still missing. After I will open some new Issues for different tasks. I hope we can recover a healthier and active workflow from now on. This long pull request in terms of time and amount of changes shouldn't be the norm 😇 |
@AeroPython/pyfme This pull request started with the objective of changing the System and Model classes (those in charge of the integration process) so that the architecture of the simulator was simpler and easier to understand. Moreover, the integration process had been reported to suffer from some problems (#89):
So EulerAnglesFlatEarth system equations have been rewritten as a single function to be integrated by Scipy. The jacobians are still to be rewritten in case we want to use them for the integration. In the meanwhile a new version of scipy came out, so the ode class has been dropped and the integration is carried out using Some classes have disappeared and many others have been created, a diagram is pending, but briefly:
I will wait one week from now (19th November) for reviews and suggestions, but I understand that under such a big amount of changes at the same time, this is a difficult task. Anyway any kind of contribution is appreciated and welcomed. 😃 |
Last commits are an answer to: #95 |
@AlexS12 Farewell my dear .py examples👋 |
@aqreed That was a great contribution (as well as the aircraft 🛩️). New examples that make the difference again are more than welcome! 👌 |
ALELUYA |
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 not just sum the winds from the Environment module? instead of the subtraction. The environment should give wind-speed vector in the Body frame, and therefore summing would look more natural?.
It's time to come back!
This pull requests introduces three main changes:
inputs_generator
has been rewritten in order to return not vectors but functions that can be evaluated at arbitrary times. @olrosalesMoreover, the documentation of more classes and methods has been added or completed (even if there are still some modifications and additions to be done)
I know this is a pretty large pull request with more than desirable changes to review at once (I apologize for that) and there are still some changes and fixes that I want to do before merging (updating the examples among them).
I have cited some of you to keep an eye on certain parts of the code and I encourage everyone (@AeroPython/pyfme) to review the state of the repository before and after this pull request so we can restart the activity with small incremental changes.