lardemua / atom

Calibration tools for multi-sensor, multi-modal robotic systems
GNU General Public License v3.0
245 stars 28 forks source link

After #882 no prior dataset collected is compatible anymore. #900

Closed brunofavs closed 3 months ago

brunofavs commented 3 months ago

Hey,

Due to now saving transform type in the dataset, in #882 , ALL old datasets are no longer compatible :

Traceback (most recent call last):
  File "/home/bruno/atom_ws/devel/lib/atom_calibration/calibrate", line 15, in <module>
    exec(compile(fh.read(), python_script, 'exec'), context)
  File "/home/bruno/atom_ws/src/atom/atom_calibration/scripts/calibrate", line 826, in <module>
    main()
  File "/home/bruno/atom_ws/src/atom/atom_calibration/scripts/calibrate", line 241, in main
    addNoiseToInitialGuess(dataset, args, selected_collection_key)
  File "/home/bruno/atom_ws/src/atom/atom_core/src/atom_core/dataset_io.py", line 818, in addNoiseToInitialGuess
    addNoiseToTF(dataset, selected_collection_key, calibration_parent, calibration_child, nig_trans, nig_rot)
  File "/home/bruno/atom_ws/src/atom/atom_core/src/atom_core/dataset_io.py", line 825, in addNoiseToTF
    if dataset['transforms'][transform_key]['type'] == 'fixed':
KeyError: 'transforms'
INFO - 2024-03-26 17:38:30,884 - core - signal_shutdown [atexit]

A script to convert old datasets into new datasets should be created.

miguelriemoliveira commented 3 months ago

Will try to fix soon ...

miguelriemoliveira commented 3 months ago

Hi @brunofavs ,

So I tried to fix but will need your help for testing.

Let me know if now you do not have this error.

The fix was this:

because of #900, and for retrocompatibility with old datasets, we will assume that if the transforms field does not exist in the dataset, then the transformation is fixed

The fix should make old datasets be possible to use again. However, the assumption for these is that the transformations are fixed, which is not your case. My feeling is that this will work for everyone else but not for you.

In any case you should try with your old dataset, the script should run but produce bad results.

brunofavs commented 3 months ago

Correct me if I'm wrong but isn't fixed and static the same? Or what is the difference?

https://github.com/lardemua/atom/blob/b98891122f17293322704b2dfd4105c869370d61/atom_core/src/atom_core/dataset_io.py#L821

Also, in spite of this fix making things work for 99% of prior cases, I reckon it introduces another hidden bug. It's true that it won't work for softbot because it will overwrite all collections with the same transformation, but the same will happen if one was to try to calibrate a transform of the UR10e for instance (hence dynamic). Granted this is highly niche considering the user would usually choose the static transform from the TCP to the sensor or something equivalent.

brunofavs commented 3 months ago

I will test it with the softbot now. I'm certain it will give awful results despite not crashing.

miguelriemoliveira commented 3 months ago

Correct me if I'm wrong but isn't fixed and static the same? Or what is the difference?

should be the same. I am trying to use fixed and abolish the static ...

brunofavs commented 3 months ago

Ok. I searched both keywords in the entire project and I did find that static was only really in a few comments not code itself.

miguelriemoliveira commented 3 months ago

Also, in spite of this fix making things work for 99% of prior cases, I reckon it introduces another hidden bug. It's true that it won't work for softbot because it will overwrite all collectionsa

right

but the same will happen if one was to try to calibrate a transform of the UR10e for instance (hence dynamic). Granted this is highly niche considering the user would usually choose the static transform from the TCP to the sensor or something equivalent.

hum, calibrating a transform from the urdf is different because those are joints.

miguelriemoliveira commented 3 months ago

Ok. I searched both keywords in the entire project and I did find that static was only really in a few comments not code itself.

great.

I will test it with the softbot now. I'm certain it will give awful results despite not crashing.

Looking forward to hear from it ...

miguelriemoliveira commented 3 months ago

Hi @brunofavs ,

this is solved, right?

brunofavs commented 3 months ago

Yes