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

Check if nig is working well #929

Open
miguelriemoliveira opened this issue Apr 18, 2024 · 7 comments
Open

Check if nig is working well #929

miguelriemoliveira opened this issue Apr 18, 2024 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@miguelriemoliveira
Copy link
Member

@fyi @brunofavs

Related to #928

Make some sanity tests to make sure noisy initial guess (nig) flag is working.

@miguelriemoliveira miguelriemoliveira added the bug Something isn't working label Apr 18, 2024
@miguelriemoliveira miguelriemoliveira self-assigned this Apr 18, 2024
@miguelriemoliveira
Copy link
Member Author

miguelriemoliveira commented Apr 18, 2024

Using nig 0 0

image

Using nig 0.1 0 we should have tranlation errors = 0.1 (Does not happen)

image

Using nig 0 0.2 we should have rotation errors = 0.2 (Does not happen)

image

brunofavs added a commit that referenced this issue Apr 18, 2024
@brunofavs
Copy link
Collaborator

brunofavs commented Apr 18, 2024

Did some more work here.

a57f165

There is still a bug somewhere.

I reckon its inside compareAtomTransforms()

With nig 0.1 0.1
I couldn't get it to fail. It seems to always be correct, which is easy to mislead us to thinking it's correct.

image

However for bigger noises, the rotation fails :

nig 10 10

image

I did some changes to make sure the noise was being computed, and it still fails with a simpler method (rather than the matrix multiplication thing"

import tf
euler_angles_init = tf.transformations.euler_from_quaternion(transform_1['quat'])
euler_angles_final = tf.transformations.euler_from_quaternion(transform_2['quat'])


deuler = np.subtract(euler_angles_final,euler_angles_init)
rotation_error = np.linalg.norm(deuler)

EDIT : With the prior matrix method to compute the rotation error the behavior was similary wrong.

@miguelriemoliveira
Copy link
Member Author

However for bigger noises, the rotation fails

bigger in rotation, translation, or both?

Is it that you are setting 10 radians for angles, which is larger that 360 degrees?

nig 10 10 is unrealistic. Does it work with nig 1 3.14?

@brunofavs
Copy link
Collaborator

brunofavs commented Apr 18, 2024

I should've been more clear.

The bigger the noise is, more often the results are wrong. At 10 rad, it's always wrong from what I tested.
What I cannot utterly understand is that for very low numbers it's accurate.

For numbers usually bigger than 2rad its inconsistent.

Runs with nig 1 3.14 :
image

First image both are wrong
2nd only one is correct
3rd only the other one is correct
4th both are correct.

I should add that the problem we were having leading to only the multiple transformations being displayed correctly I solved already ( in the commit referenced in my previous comment)

@brunofavs
Copy link
Collaborator

brunofavs commented Apr 18, 2024

It is true that 10rad is outrageous and unrealistic, but it still makes me wonder why it is happening.

While it does not seem to happen for more realistic lower values, I cannot assure that it wont happen given a big enough batch experiment. I might ve simply not ran enough runs to experience it yet .

@miguelriemoliveira
Copy link
Member Author

It is true that 10rad is outrageous and unrealistic, but it still makes me wonder why it is happening.

I understand your discomfort, but perhaps we can take a more practical approach.
Test it and see if it works fine until 0.5 3.14/2.
If it does, lets assume the error cannot be larger than this. We set a validation on the add noise that throws an atom error saying error cannot be larger than X.

brunofavs added a commit that referenced this issue Apr 24, 2024
- Bigger noises are still bugged but now gives a atomWarn()
- Encapsulated ntfv in a specific function
- -ntfv now overrides -nig
- addNoiseToTF now calls a subfunction computeNoise() to reduce boilerplate code
- other misc small qol improvements
@brunofavs
Copy link
Collaborator

from 576b626

  • Bigger noises are still bugged but now gives a atomWarn()

It's not solved but it will be a lower priority.

The issue will remain open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants