lardemua / atom

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

'inspect_atom_dataset' checks for all the patterns (when there are multiple) to decide if a collection is partial or not #892

Closed Kazadhum closed 3 months ago

Kazadhum commented 3 months ago

Hello! I was using the inspect_atom_dataset script to check for partial detections of patterns in the riwmpbot_real dataset. I found this:

https://github.com/lardemua/atom/blob/5a2c1d6e5d7e3161c1f18ff73910928d7df1c717/atom_calibration/scripts/utilities/inspect_atom_dataset#L98-L109

As it stands, when there are multiple patterns, this checks if all the labels of every patterns are detected for each collection. I assume we want it to check if a collection has detected all the corners for each pattern, instead of all at once (since we print out a table for each pattern).

Kazadhum commented 3 months ago

gonna try to fix this now

Kazadhum commented 3 months ago

Done.

miguelriemoliveira commented 3 months ago

Hello! I was using the inspect_atom_dataset script to check for partial detections of patterns in the riwmpbot_real dataset. I found this:

https://github.com/lardemua/atom/blob/5a2c1d6e5d7e3161c1f18ff73910928d7df1c717/atom_calibration/scripts/utilities/inspect_atom_dataset#L98-L109

As it stands, when there are multiple patterns, this checks if all the labels of every patterns are detected for each collection. I assume we want it to check if a collection has detected all the corners for each pattern, instead of all at once (since we print out a table for each pattern).

Hi @Kazadhum ,

well the concept of partial detection was created when we had a single pattern.

But I think we can adapt and make it a per pattern concept, meaning a partial detection is the partial detection of a specific pattern.

Not sure what you mean by this ...

I assume we want it to check if a collection has detected all the corners for each pattern, instead of all at once (since we print out a table for each pattern).

Is it the same that I am suggesting? Note that the concept is transversal to atom, and therefore the justification should not be "because in these tables we are printing in this script it makes more sense like this".

Another suggestion is to create a function somewhere in atom core and use it in the script.

Kazadhum commented 3 months ago

Hello @miguelriemoliveira! I think you put it in simpler terms than I did!

But I think we can adapt and make it a per pattern concept, meaning a partial detection is the partial detection of a specific pattern. Is it the same that I am suggesting? Note that the concept is transversal to atom, and therefore the justification should not be "because in these tables we are printing in this script it makes more sense like this".

Effectively I did exactly what you suggested. If you try to use this script on the riwmpbot datasets now, it does a "per pattern" analysis.

In fact, the logic of the script already seemed to be that, since it iterated through the list of patterns and printed a table for each pattern. What was on those tables was wrong, though, and that was what I fixed. I agree that the justification shouldn't be based on how it was implemented before but should have the entirety of ATOM in mind, but in this case I think the two are aligned.

miguelriemoliveira commented 3 months ago

Great. We are aligned.

Before I said:

Another suggestion is to create a function somewhere in atom core and use it in the script.

I meant a function that receives a detection and returns True or False to signal if its partial or not.