Noblis / INVSC-janice

An open API for computer vision algorithms
https://noblis.github.io/janice/
Other
9 stars 4 forks source link

Repeated line items getting processed #32

Closed carlosdcastillo closed 6 years ago

carlosdcastillo commented 6 years ago

When you print here: https://github.com/Noblis/janice/blob/6.0/harness/janice_enroll_detections.cpp#L113

the filename being processed, you should not see repeated filenames, and you should never see repeated images (from the img/ directory) you should only see frames. If you're processing repeatedly the same line you're wasting time and resources. On average each line appears three times.

carlosdcastillo commented 6 years ago

Upon further review I believe that unless I'm missing something fundamental, my previous posting is a symptom, not the disease. The disease is that the current janice_enroll_detections will not generate all the templates in the csv file. This is not an efficiency thing, it is a correctness problem.

This line https://github.com/Noblis/janice/blob/6.0/harness/janice_enroll_detections.cpp#L87

assumes that the same sighting_id can only appear in one template, this is assumption is not correct.

carlosdcastillo commented 6 years ago

What is the status on this? Can you confirm that you can reproduce the issue?

Unless I'm missing something, with this problem it is impossible to reliably create templates from detections using the harness. Creating templates is needed for the vast majority of tasks.

JordanCheney commented 6 years ago

@carlosdcastillo I've reproduced the issue. This is a problem with how we parse the CSV as you noted. Because we don't have a compliant SDK for 6.0 there are a few bugs like this remaining in the harness. I've ported a reference implementation so I can do proper testing. I will have patches for this and the other outstanding harness issues today.

JordanCheney commented 6 years ago

@carlosdcastillo c1fc062 should fix this issue. Can you confirm when you have the chance?

carlosdcastillo commented 6 years ago

Thanks! I ran enrollments the over night. They ran, I will check then and let you know today PM.

carlosdcastillo commented 6 years ago

it says image, it should say probe template: https://github.com/Noblis/janice/blob/6.0/harness/janice_search.cpp#L21

it says image, it should say gallery template: https://github.com/Noblis/janice/blob/6.0/harness/janice_create_gallery.cpp#L19

They should probably not be optional.

carlosdcastillo commented 6 years ago

With respect to timing, what exactly does that does that mean:

  1. Timing is not necessary for this data call
  2. Timing will be fixed for the harness
  3. Timing will be necessary for the data call and won't be included in the harness, solve the problem as implementer sees fit.
JordanCheney commented 6 years ago

@carlosdcastillo The argument changes you suggested are implemented in 49e33bf. I made the prefix path required in all the executables for consistency

JordanCheney commented 6 years ago

Timing is necessary for this data call. The batch calls make timing individual things impossible in the harness, so I didn't include it. However, I think we can update the harness to measure average times over batches. We are discussing internally tomorrow AM to make sure this satisfies our test requirements. I will post an update as soon as we make a determination.

JordanCheney commented 6 years ago

@carlosdcastillo @taa01776 For timing, we are going to output per-batch runtimes in the harness. I am going to add an output column BATCH_IDX to each output file along with the runtime for that batch. An example would be-

TEMPLATE_ID,FILENAME,...,BATCH_IDX,*_TIME(ms)
0,file1.png,...,0,4132
0,file2.png,...,0,4132
1,file3.png,...,1,8354
1,file3.png,...,1,8354

Harness update to follow shortly.

JordanCheney commented 6 years ago

Done in 8b4a9d2