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

Improve object detection task guideline #29967

Merged

Conversation

NielsRogge
Copy link
Contributor

What does this PR do?

This PR fixes some issues mentioned in #29964

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@NielsRogge NielsRogge requested a review from amyeroberts April 1, 2024 07:38
Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

Just some small nits & and a q about timm

docs/source/en/tasks/object_detection.md Outdated Show resolved Hide resolved
docs/source/en/tasks/object_detection.md Outdated Show resolved Hide resolved
docs/source/en/tasks/object_detection.md Show resolved Hide resolved
@@ -362,7 +363,7 @@ Face to upload your model).
>>> training_args = TrainingArguments(
... output_dir="detr-resnet-50_finetuned_cppe5",
... per_device_train_batch_size=8,
... num_train_epochs=10,
... num_train_epochs=100,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you manage to investigate this? 100 epochs is very large. If we train for this many steps do we still see a change in metrics over the first 10 epochs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @qubvel perhaps this would be great to investigate as part of making it easier to train object detection models

@NielsRogge
Copy link
Contributor Author

@amyeroberts could we already merge this PR which addresses the first 3 points of #29964?

@amyeroberts
Copy link
Collaborator

@NielsRogge Did you run and check re 100 epochs? #29967 (comment) If not, then we should revert back until confirmed

@qubvel
Copy link
Member

qubvel commented Apr 30, 2024

With 100 epochs it converges better but still has pretty low metrics. Also, predictions are bad because we use it for inference with batch size = 1.
We can align it with #30422 when it's merged to get better metrics.

@NielsRogge
Copy link
Contributor Author

NielsRogge commented May 1, 2024

@amyeroberts people have reported to replicate it with 100 epochs here: #30557 (comment)

Feel free to approve the PR in order to clarify this for now in the docs. Later on we could improve metrics more

Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks for improving our task guides!

@NielsRogge NielsRogge merged commit dc401d3 into huggingface:main May 1, 2024
8 checks passed
itazap pushed a commit that referenced this pull request May 14, 2024
* Add improvements

* Address comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants