agentmorris / MegaDetector

MegaDetector is an AI model that helps conservation folks spend less time doing boring things with camera trap images.
MIT License
103 stars 24 forks source link

Consider commits from CameraTraps/pull/349 #95

Closed agentmorris closed 1 year ago

agentmorris commented 1 year ago

@bobbycephy created microsoft/CameraTraps/pull/349 ("Improvements and extensions of run_detector.py and visualization_utils.py"), considering those changes for this repo.

These changes primarily extend run_detector.py, to the point that may exceed the spirit of run_detector.py (almost exclusively used as a simple test script), into functionality that is better supported by run_detector_batch.py. This PR would improve support for some non-camera-trap-related scenarios, at the cost of some complexity for core camera trap scenarios. Specific changes include:

agentmorris commented 1 year ago

Cherry-picked the commit about missing arguments to crop_image, not pulling in the others for now. Thanks for your contribution!

BobbyCephy commented 1 year ago

@agentmorris Thank you for creating an issue considering my pull request! My changes to run_detector.py should only provide some useful optional features without affecting the current functionality as shown in the examples. I can't think of a better place for these features than run_detector.py, since they are only meant to generate improved output images from a few input images. In contrast, run_detector_batch.py only outputs detection results as a json file. Or in which way output images are intended to be generated based on those results? My changes should improve support for camera trap scenarios as well with negligible increase in complexity. They were just inspired by manual photography.

agentmorris commented 1 year ago

I think these are all just a little out of scope for our core scenario... generating new images (cropped or otherwise) is already a very rare scenario for camera trap data, and in the only case where it's necessary to copy metadata for that scenario, it's already handled. And without a compelling scenario, I'm really hesitant to rock the boat on functions like load_image() that are called hundreds of millions of times per year. I'm not deleting this issue, I'm just not going to pull any of these changes in right now; if any of these come up for camera trap scenarios, I will refer back to this issue.

And re: the --image_file changes... while your suggestion is very reasonable, we also try not to rock the boat on command-line syntax, since the substantial majority of our users have no experience in Python. And since "python run_detector.py --image_file image.jpg model_file.pt" is perfectly legal right now, it becomes hard to communicate about the behavior of the command line if --image_file can be followed by multiple arguments. I honestly had to look up what happens in this scenario. :) So for clarity, I would prefer to avoid nargs="*" here. run_detector_batch.py is the preferred method for bulk inference, and run_detector_batch.py can take a list of files as input.

But thanks again for your contribution (especially the bug fix)! It sounds like you may be working on a project related to wildlife photography more broadly, and it would be great to hear that MegaDetector may have applications there as well. I would call to your attention some of the limitations of training on camera trap data which could be problematic for non-camera-trap scenarios (particularly unexpected failures on cold-blooded animals), but if it works, it works!