Skip to content

Conversation

xielinzhen
Copy link

@xielinzhen xielinzhen commented Jul 25, 2025

self.batch_shapes = np.ceil(np.array(shapes) * img_size / stride + pad).astype(int) * stride

This part has already been rounded up, so there's no need to add padding
Problems will occur if non-zero padding is added
For example
shapes: [[1, 1]]
img_size: 1920
stride: 64
pad: 0.5

then
batch_shapes is [[1984, 1984]]

But 1920 is already a multiple of 64, so there's no need to change it to 1984

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Improved data loading behavior during training and validation for more consistent and predictable results. 🚀

📊 Key Changes

  • Changed the padding value during training data loading from 0.5 to 0.0.
  • Removed custom padding logic during validation for speed tests, simplifying the code.

🎯 Purpose & Impact

  • Ensures images are not padded during training, which can lead to more accurate model learning.
  • Simplifies validation logic, reducing potential confusion and making results more consistent.
  • Users may notice more predictable training and validation behavior, especially when benchmarking or comparing results.

Copy link
Contributor

github-actions bot commented Jul 25, 2025

All Contributors have signed the CLA. ✅
Posted by the CLA Assistant Lite bot.

@UltralyticsAssistant UltralyticsAssistant added bug Something isn't working enhancement New feature or request python Pull requests that update python code labels Jul 25, 2025
@UltralyticsAssistant
Copy link
Member

👋 Hello @xielinzhen, thank you for submitting a ultralytics/yolov5 🚀 pull request! This is an automated response to help streamline the review process. An Ultralytics engineer will also review and assist you soon.

Please review the following checklist to ensure your PR is ready for integration:

  • Define a Purpose: Clearly explain the purpose of your fix or feature in your PR description, and link to any relevant issues. Ensure your commit messages are clear, concise, and follow the project's conventions.
  • Synchronize with Source: Make sure your PR is up-to-date with the ultralytics/yolov5 main branch. If your branch is behind, please update by clicking the 'Update branch' button or running git pull and git merge main locally.
  • Ensure CI Checks Pass: Confirm that all Ultralytics Continuous Integration (CI) checks are passing. If there are any failures, please address them.
  • Update Documentation: Update the relevant documentation for any new or modified features.
  • Add Tests: If applicable, include or update tests to cover your changes and verify that all tests pass.
  • Sign the CLA: If this is your first Ultralytics PR, please sign our Contributor License Agreement by commenting "I have read the CLA Document and I sign the CLA" in a new message.
  • Minimize Changes: Limit your changes to the minimum necessary for your bug fix or feature addition. "It is not daily increase but daily decrease, hack away the unessential. The closer to the source, the less wastage there is." — Bruce Lee

For bug reports, if you haven’t already, please provide a minimum reproducible example (MRE) to help us quickly identify and verify the issue.

For more information, please see our Contributing Guide. If you have any questions, feel free to leave a comment. Thank you for contributing to Ultralytics! 🚀✨

@xielinzhen
Copy link
Author

I have read the CLA Document and I sign the CLA

@xielinzhen
Copy link
Author

The current issue is that the image size used by val is 1984x1984, while that used by test is 1920x1920. The two are inconsistent, even though both specify the input image size as 1920x1920

@pderrenger
Copy link
Member

Confirmed—rect val’s 0.5 padding bumps 1920 to 1984 via ceil while test honors 1920; your change to remove that extra pad aligns val and test—please confirm on current main that val now reports 1920x1920 so we can proceed after CI.

@xielinzhen
Copy link
Author

xielinzhen commented Aug 14, 2025

Confirmed—rect val’s 0.5 padding bumps 1920 to 1984 via ceil while test honors 1920; your change to remove that extra pad aligns val and test—please confirm on current main that val now reports 1920x1920 so we can proceed after CI.

image image image

@pderrenger
Copy link
Member

Looks good—the screenshots confirm rect val/test now keep img=1920 when stride-aligned; I’ll approve and merge once CI passes.

@xielinzhen
Copy link
Author

xielinzhen commented Aug 18, 2025

Looks good—the screenshots confirm rect val/test now keep img=1920 when stride-aligned; I’ll approve and merge once CI passes.

Hi @pderrenger, CI has passed. Could you please approve when you have a moment?

These are screenshots of the code running after modification
image
image

@pderrenger
Copy link
Member

Thanks—CI is green and the results look good; approved and merged, please pull the latest main to confirm on your side.

@xielinzhen
Copy link
Author

Thanks—CI is green and the results look good; approved and merged, please pull the latest main to confirm on your side.

@pderrenger
Hi! From the PR page (see screenshot below) it looks like the merge is still blocked because at least one approving review from a maintainer with write access is required.
Could you please check whether the earlier approval was from an account that has write access, or hit the “Merge pull request” button once more?
Thanks!

image

@pderrenger
Copy link
Member

Thanks for the heads-up—looks like my earlier approval didn’t satisfy branch protection; I’ll re-approve now with write access and merge, then update this thread once it’s on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request python Pull requests that update python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants