Skip to content
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

Tilt rotor model #70

Merged
merged 14 commits into from May 17, 2021
Merged

Tilt rotor model #70

merged 14 commits into from May 17, 2021

Conversation

manumerous
Copy link
Contributor

@manumerous manumerous commented May 9, 2021

First PR of the Tilt rotor model.

The model still does not work very nicely and I am still debugging it. But this new model and the functionalities are getting so large that we decided to nonetheless start merging this PR.

@manumerous manumerous marked this pull request as draft May 9, 2021 13:12
@manumerous manumerous force-pushed the tilt-rotor-model branch 4 times, most recently from 1a0cd49 to 00432b3 Compare May 14, 2021 19:59
…mation

In this commit the calculation of the rotor features was moved to a method in the dynamics model and is therefore now unified between all the models. The user can configure rotor groups in the config file of his model. Hereby, all rotors from the same group are assumed to be physically (motor, prop, voltage, ect.) equal and will share all coefficients as determined by the parametric model.

Furthermore, i decided to allow the user to configure whether forces, moments or both are estimated by the algorithm. This was combined with the changes mentioned above since the rotor force and moment features share a significant part of the computation. In particular both need to compute the airplow parallel and perpendicular to the rotor axis for each timestamp. With the current implementation those computations are shared when both are activated which results in much faster performance for large datasets.
@manumerous manumerous force-pushed the tilt-rotor-model branch 2 times, most recently from f2e8fa5 to da55de8 Compare May 15, 2021 16:50
@manumerous manumerous force-pushed the tilt-rotor-model branch 2 times, most recently from a7a1812 to 041504f Compare May 16, 2021 13:53
@manumerous manumerous marked this pull request as ready for review May 16, 2021 15:49
@manumerous manumerous changed the title [WIP] Tilt rotor model Tilt rotor model May 16, 2021
Copy link
Member

@Jaeyoung-Lim Jaeyoung-Lim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the pyc being added and removed among the commits?

This will just increase the repo size, given that you don't want the merge to be squashed?

Copy link
Member

@Jaeyoung-Lim Jaeyoung-Lim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think probably you understand the code better than I do, and since this is a WIP branch it looks like this should be good to go.

However, it seems like this is NOT a PR for the tiltrotor model, but a lot more and it is not transparent from this PR.

  • Adding the dataframe selector
  • Adding a tiltrotor model with change axis rotor model
  • Addition of bidirectional rotor model (probably related to tiltwing)
  • Some cleanups including path changes for quat_utils

Thanks for taking the time to write commit messages, but the PR description only talks about the status of the tiltwing and this makes the changes somewhat intransparent.

Tools/parametric_model/generate_parametric_model.py Outdated Show resolved Hide resolved
.gitmodules Outdated Show resolved Hide resolved
@manumerous
Copy link
Contributor Author

@Jaeyoung-Lim

No you are completly right to point out that there were a few more things done than just adding the tiltwing model. Maybe I could have made a better plan of what exactly i need in the beginning to keep the functionalities more separated. Sorry for not mentioning that in the PR text.

@manumerous
Copy link
Contributor Author

I think I resolved the two change requests you made. Am I missing something else?

@Jaeyoung-Lim
Copy link
Member

@manumerous So as far as I understood, you want to leave to pycache in?

@manumerous
Copy link
Contributor Author

manumerous commented May 17, 2021

As sorry it kind of posted my answer in the wrong place i saw. Here it is again:

Could you remove the pyc being added and removed among the commits?

This will just increase the repo size, given that you don't want the merge to be squashed?

Yes I saw that too late that it was there. Since we will most likely publish the repo without the commit history It is probably not worth redoing all the commit history for this because the file is gone now. Is just in the history. I prefer not to in that case.

However, I already added all *.pyc to the gitignore file a while ago. So I am not sure why it still showed up again. Maybe it was already present before and i forgot to delete it?

@Jaeyoung-Lim
Copy link
Member

Jaeyoung-Lim commented May 17, 2021

@manumerous I replied to that comment, I don't see why you need to redo all the commit history. If you think it is worth getting in lets do it. However I am not sure why removing it will take more than a few minutes.

I am also slightly confused about the repo being squashed - why are you writing the commit messages then?

@manumerous
Copy link
Contributor Author

Okay how else would you do this then instead of resetting to the commit the file was added and then redoing all commits?

I think at some point we said that we would squash the repository when we make it public. I was referring to that and not to this PR.

@Jaeyoung-Lim
Copy link
Member

@manumerous As per my reply in the comments:

You don't need to redo the commit history. Just branch off where the pyc files were added, remove it and rebase this branch on top of it - there will be no merge commits.

This commit fixes a mistake in the aero model for delta and AAE configuration. Before the fix the drag force (acting in x axis of the stability axis frame) would have switched its direction depending on the sign of the angle of attack.
I added the possibility to specify the density and prop diameter for each rotor model. If they are not specified both attributes will be set to 1 and a lumpsum parameter including those properties will be estimated. So this option leaves all possibilities open.
Since we anyways have a single input vector per rotor it was made a property of the Rotormodel instead of passing it separatly for estimating forces and moments. This will also be helpfull when passing the rotor as an argument for airfoil sections in the downwash of the rotor for the tiltwing model.

Furthermore the initialization of the airspeed is now called in the init function, since it anyways needs to happen before any methods of the class can be used.
When the user decides to pass a matrice of angluar velocities corresponding with its respective entries in the airspeed matrix a new local airspeed for each rotor is computed by adding the cross product of angular velocity and rotor position to the airspeed.
A method was added to predict the thrust force inside the RotorModel. This will be used in the future to estimate the propeller downwas for the tiltwing model.
Since i guess that numpy arrays are mutable objects and python uses Pass-by-object-reference this could lead to modification of the original memory localtion where the original values of v_airspeed_mat are stored. Here I am not quite sure. But this new way for sure safeguards us against the manipulation of the original values.
The check, whether all required topic types and topics are present in a ulog file was adapted to give detailed feedback on which topic or topic type is missing.
Commit adapts the standard rotor model to specify the rotor axis (direction of thrust force) for each computation of the force or moment feature matrix. This is needed to make the model combatiple with with its childs. In the standard model always the fix rotor axis is used for this computation.

The changing axis rotor model is a child of the rotor model and introduces a matrix of rotor axis where each row represents the axis for the corresponding timestep. This model itself is not intended to be in a model, as can be seen by the fact, that it just constructs the matrix by all identical rotor axis vectors. Instead it is ment as a common basis for the other tho new models.

The bi-directional rotor model is a child of the changing axis rotor model. Hereby the rotor can turn in both directions which subsequentially results in the thrust force changing direction. The model handels this, by expecting a normalized actuatir input vector between [-1, 1]. Hereby, the sign of the entry is used to switch the force directions for the negative inputs.

Last but not least a Tilting Rotor model is introduced. This model can be used to model rotors, where the direction of the rotoraxis can be adjusted by a separate actuator as can be done in tiltwings or tiltrotor models. Also this model is a child of the changing axis model and overrides the function for the creation of the rotor axis matrix.
This commit adds a tilt wing model, including config and aero model for each tilt wing section and a few other adaptions needed to make the model usable.
All inputs are now absolute. The __init__.py files and ulog_utils were cleaned up.
The user can choose to run the Visual Dataframe Selector before the estimation beginns to select a subportion of the data. This can be done by passing optional arguments in both the makefile or directly to the python file.
@manumerous manumerous merged commit 6e66cc3 into master May 17, 2021
@manumerous manumerous deleted the tilt-rotor-model branch May 17, 2021 10:55
@Jaeyoung-Lim
Copy link
Member

Jaeyoung-Lim commented May 17, 2021

Still went in 😅 766ee9d

Lets just move on

@manumerous
Copy link
Contributor Author

manumerous commented May 17, 2021

I think did as you suggested. There is now an additional commit that removes them directly after they were added. I guess i would have had to reset to before 766ee9d and redo all the commits to make this work. Anyways this commit is strange... it does not contain any changes to the code and does nothing but adding the pyc files.... I guess there went something wrong with a rebase or similar.

But now i have a tone of merge conflicts in my next branch due to this... So i think it was not worth all the time invested...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants