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

Add configurable max_hamming_distance for the AprilTag Detector #93

Merged

Conversation

akshayprsd
Copy link

Adds configurable max hamming distance bits for the AprilTag Detector.

Context:

  1. Noticed high memory usage for the 52h13 family detector.
  2. Noticed that apriltag_ros initiates the apriltag detector with max_hamming_distance value of 2 bits by default which is hardcoded in the apriltag library.
  3. Noticed that reducing the max_hamming_distance bits from 2 to 1 brings down memory usage drastically.
  4. Added configurable ros param to be able to configure this value.

I might have missed some documentation files which might need to be updated. Happy to take a look at those.

@akshayprsd
Copy link
Author

@wxmerkt Can we get this merged if its good to go?

@akshayprsd akshayprsd force-pushed the add_configurable_max_hamming_distance branch from 4793e17 to e00abe9 Compare April 6, 2021 01:08
Based on upstream code comment:
> Tunable, but really, 2 is a good choice. Values of >=3
> yonsume prohibitively large amounts of memory, and otherwise
> you want the largest value possible.
@wxmerkt
Copy link
Collaborator

wxmerkt commented Apr 6, 2021

I added comments on the parameter setting from upstream which are now passing through the buildserver. I am not certain if there isnt a loss of quality of detection reducing the setting? How much was the memory usage before/after?

@akshayprsd
Copy link
Author

The memory usage drops from about ~6 GB for the 52h13 family to ~ 200 MB.

int capacity = family->ncodes; // 48714
int nbits = family->nbits;
if (maxhamming >= 1)
   capacity += family->ncodes * nbits; // 48714 + 48714 * 52
if (maxhamming >= 2)
   capacity += family->ncodes * nbits * (nbits-1); // 48714 + 48714 * 52 + 48714 * 52 * 51
qd->nentries = capacity * 3; // (48714 + 48714 * 52 + 48714 * 52 * 51) * 3
qd->entries = calloc(qd->nentries, sizeof(struct quick_decode_entry)); // (48714 + 48714 * 52 + 48714 * 52 * 51) * 3 * 12

So memory allocated = (48714 + 48714 * 52 + 48714 * 52 * 51) * 3 * 12 = 4743769320 bytes ~ 4.73 gigabytes with max_hamming_distance = 2.

For max_hamming_distance = 1 -> ((48714) + (48714 * 52)) * 3 * 12 = 92946312 bytes ~ 92 MEGAbytes.

If we reduce the max_hamming_distance it should increase the accuracy as we are allowing fewer bit errors to be looked up. However some valid detections might dropped because we are allowing fewer errors.

Irrespective I think being able to configure the parameter is valuable and we retain the original functionality by setting 2 as the default parameter.

@wxmerkt wxmerkt merged commit acfce85 into AprilRobotics:master Apr 7, 2021
@wxmerkt
Copy link
Collaborator

wxmerkt commented Apr 7, 2021

Thank you for the detailed explanation and contribution. :-)

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