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

RotorModel fails to calculate relevant features if position and axis vector are collinear #171

Open
pascalau opened this issue Oct 25, 2021 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@pascalau
Copy link
Collaborator

pascalau commented Oct 25, 2021

leaver_moment_vec = np.cross(
self.rotor_position.flatten(), rotor_axis.flatten())

If the two parameters rotor_position and rotor_axis are collinear the resulting features are all zero. This can cause issues in the regression since the features are all non descriptive.

@manumerous
Copy link
Contributor

Hi Pascal,

Thanks for reporting the issue. Yes you are right that this would result in the feature being zero which could lead to problems depending on how the optimizer initializes the optimization coefficients.

I agree that it makes sense to safeguard against this behavior. I would propose to add a general test as a function for this in the optimizer base template. Hereby we could test each row of the feature matrix X for having non descriptive features and setting all corresponding coefficients to zero.

Would you like to add a PR this in a new PR?

@manumerous manumerous self-assigned this Oct 26, 2021
@pascalau
Copy link
Collaborator Author

pascalau commented Oct 29, 2021

Sorry for responding so late (busy week). After analyzing the problem I have the following proposition:
The issue can arise in two cases:

  • The model is ill posed (in this example there is no lever for this rotor)
  • The actuator is never actuated in the log.

In my opinion there should be two changes to address this issue:

  • The Model should not produce ill-posed features. If it detects for example that the crossproduct is zero it should omit these features since the result will be nonsense anyway. This should apply to all other models that produce features.
  • The optimizer should also check for correct features and warn the user that the result for this specific parameter will be undefined. I don't think that just returning zero is the proper way to handle this case, since the program should indicate to the user that something went wrong. I would rather suggest returning something like NAN or None if this happens.

@manumerous
Copy link
Contributor

manumerous commented Oct 29, 2021

To your problem analysis:

I do not think that the model automatically ill posed when there is no leaver moment for a certain rotor. If we think about a fw aircraft with only a single rotor it is actually desirable to not have a moment contribution of this rotor (Or at least be very close to zero contribution.

To your proposed changes:

  • I do not think including branching in the amount of model features (e.g. reducing the amount of coefficients when they have no influence) is the way to go here. This will just result in us needing to check whether every component is present in the model output yaml file in the gazebo implementation of every model. So if it is the case that a coefficient has no influence on the output it would be correct and the easiest to let the optimizer return 0 I think.

  • Yes that is a very good point! We definitely need that for the case where the rotor is never actuated in the log! I have been thinking about making some kind of PDF report that includes significance and correlation metrics for each coefficient. This way it would be more information than just a binary classification. But that might take some time to implement so your suggestion would certainly be a step in the right direction.

How would you suggest to determine whether a feature is ill posed?

@pascalau
Copy link
Collaborator Author

pascalau commented Nov 1, 2021

I do not think that the model automatically ill posed when there is no leaver moment for a certain rotor. If we think about a fw aircraft with only a single rotor it is actually desirable to not have a moment contribution of this rotor (Or at least be very close to zero contribution.)

I agree. I think it is very important to differentiate between the case of the feature is zero due to geometry or due to actuator inactivity during the logging and should be handled differently. If dropping the feature causes issues later on in the pipeline I would consider setting the parameter to zero if the cause is a geometrical problem. A solution would be that the model could set a flag or prematurely set the result of the parameter to zero before the optimizer does it's work. And if the optimizer detects zero columns in the feature matrix it can check if the parameter is already defined and in this case proceed with optimization or abort and report that the problem is ill-posed.

@pascalau
Copy link
Collaborator Author

#174 Implements a brute force check to look for invalid features given to the optimizer. The handling of invalid feature due to geometry still has to be implemented

@Jaeyoung-Lim Jaeyoung-Lim added the bug Something isn't working label Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants