API inconsistency between image and video prediction

[laszlovandenhoek]

🚀 Feature Request

If I run predict() on an image, as per the YOLO-NAS quickstart, it yields an ImagesDetectionPrediction, which has a save() method that takes an output_folder argument:

https://github.com/Deci-AI/super-gradients/blob/a59449726e1d55f68c96b7be24f8a00a126b59e0/src/super_gradients/training/models/prediction_results.py#L182-L184

However, if I run that same predict() method on a video, it yields a VideoDetectionPrediction, whose save() method takes an output_path argument:

https://github.com/Deci-AI/super-gradients/blob/a59449726e1d55f68c96b7be24f8a00a126b59e0/src/super_gradients/training/models/prediction_results.py#L237

Considering that they mean the same thing, it was confusing to me that the method arguments are named differently, especially since it's a required parameter without a default value.

Proposed Solution (Optional)

• add a save() method to VideoDetectionPrediction that takes an output_folder argument
• deprecate the existing one that takes the output_path argument
• ideally, refactor so that the original filename is used as a basis to build the new filename. Using .save(output_path="") (by analogy of the quickstart doc) currently saves the output to a file literally named .mp4.

The reason I would suggest replacing the save() method on VideoDetectionPrediction instead of the one on ImagesDetectionPrediction is that the quickstart documentation is based on an image and therefore uses the output_path form.

[dagshub[bot]]

Join the discussion on DagsHub!

[Louis-Dupont]

Hi @laszlovandenhoek , thanks for your feedback

To add some context, the reason why both have different name is because they refer to something slightly different.

• output_folder is the folder where all the images will be saved under <output_folder>/f"pred_{i}.jpg". This is used for multiple images ImagesDetectionPrediction

• output_path refers to the path where the video will be saved. This includes the name of the video and the extension. This is used for video VideoDetectionPrediction and single image prediction ImageDetectionPrediction

Motivation for current implementation

The motivation is that when saving a single object (video or image), we can let the user set the name (with output_path) while when working with a set of images, each image needs to have a different name, and therefore we chose to let the use chose the folder only (with output_folder). Eventually, we do want to improve the logic of multiple images, because currently, the user has no control over the name of each image (i.e. f"pred_{i}.jpg).

Potential Solution

Concerning your solution, I am not sure if taking out the possibility of the user to choose the output video/single image name is a good thing. That being said, I totally agree that homogenizing the API would be great.

Maybe the right approach would be something similar to this:

• Keep multiple images predictions (ImagesDetectionPrediction) as is
• Add output_folder to video (VideoDetectionPrediction) and single image (ImageDetectionPrediction) .save method.
• Deprecate output_path from both classes (Non-relevant if we already have output_folder)
• Add output_name to both classes as optional, to let the user chose a name. By default, output_folder="predicted_video.mp4" or output_folder="predicted_image.jpeg" depending on the class

ImagesDetectionPrediction(...).save(output_folder: str, ...)
ImageDetectionPrediction(...).save(output_folder: str, output_name: str = "predicted_image.jpeg", ...)
VideoDetectionPrediction(...).save(output_folder: str, output_name: str = "predicted_video.mp4", ...)

What do you think?

[laszlovandenhoek]

Fair enough. While we're changing method signatures, I think it would make sense to use this opportunity to also add the possibility to customize the image name in the case of ImagesDetectionPrediction. I'm no Python wizard, but perhaps something like this could work:

from typing import Callable
...
ImagesDetectionPrediction(...).save(output_folder: str, file_naming_strategy: Callable[[int], str] = lambda i: f"pred_{i}.jpg", ...)

That would maintain backwards compatibility while providing customizability going forward.

[Louis-Dupont]

Update; If you want to save each image with a clear and custom name, the recommended way is to simply iterate over the predictions, and save each with whatever name you want

predictions = model.predict(IMAGES)

for i, prediction in enumerate(predictions):
    name = ...  # Define the name the way you want. e.g. `name = f"my_custom_name_{i}"`
    prediction.save(output_path=name)

(As opposed to the quick way)

predictions = model.predict(IMAGES)
predictions.show()
predictions.save(output_folder="")  # Save in working directory