Skip to content

Conversation

@mayocream
Copy link
Contributor

Fix #1858

Copy link

@sdabbour sdabbour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been diving into the code recently to investigate implementation of an example for Qwen 3 VL. These are just some things I noticed from a layout perspective.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this model be better placed in the candle-transformers/src/models/ dir?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, yolov5 is quite old, it's ok to implement your own models for whoever needs it based on candle-examples?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is necessary to include, could maybe describe a bit how to get the model in the README.md file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably fine for a small stand-alone python script to be included, especially one that is somewhat obvious like this one.

if there's anything I'd change, is to maybe make it stand-alone executable with uv by including dependencies in the script itself: https://docs.astral.sh/uv/guides/scripts/#declaring-script-dependencies

then one can just uv run script.py without much thinking.

Copy link
Contributor

@slckl slckl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yolov5 is quite old by now, but still surprisingly popular. I've confirmed the present impl works.

Would like to see output logs + classes/confidences in output annotations, and this would be a fine addition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably fine for a small stand-alone python script to be included, especially one that is somewhat obvious like this one.

if there's anything I'd change, is to maybe make it stand-alone executable with uv by including dependencies in the script itself: https://docs.astral.sh/uv/guides/scripts/#declaring-script-dependencies

then one can just uv run script.py without much thinking.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, yolov5 is quite old, it's ok to implement your own models for whoever needs it based on candle-examples?

## Running an example

```bash
$ cargo run --example yolo-v5 --release -- candle-examples/examples/yolo-v8/assets/bike.jpg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm this works and produces sane-looking output 😃

),
image::Rgba([255, 0, 0, 255]),
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please include class labels + confidence on the output image? similar to how yolo-v8 example does it?

I would also be grateful if you println logged the class names + bbox data similar to how yolo-v8 example does it? It's very useful to validate any numerical changes in the library etc:

            println!(
                "{}: {:?}",
                candle_examples::coco_classes::NAMES[class_index],
                b
            );

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.

Will candle support YOLOV5?

3 participants