Skip to content

warn and disable training resize-and-crop branch when aug_config={}#1037

Open
omkar-334 wants to merge 16 commits into
roboflow:developfrom
omkar-334:fix-resize-crop
Open

warn and disable training resize-and-crop branch when aug_config={}#1037
omkar-334 wants to merge 16 commits into
roboflow:developfrom
omkar-334:fix-resize-crop

Conversation

@omkar-334

@omkar-334 omkar-334 commented May 12, 2026

Copy link
Copy Markdown
Contributor

Title: warn and disable training resize-and-crop branch when aug_config={}


What does this PR do?

Setting aug_config={} does not stop the default random resize-and-crop applied to training images. This is structurally correct but counter-intuitive: aug_config and the resize/crop logic live in two separate stages of the training pipeline, and aug_config only controls one of them.

In make_coco_transforms (src/rfdetr/datasets/coco.py), the training pipeline composes two stages:

  1. Resize stage. Built from _build_train_resize_config(...). It hard-codes a OneOf between:
    • Option A: direct resize to the target scale.
    • Option B: SmallestMaxSize([400, 500, 600]), then RandomCrop/RandomSizedCrop, then resize to the target scale.
  2. Augmentation stage. Built from aug_config (color jitter, flips, rotations, etc.).

aug_config={} zeroes out stage 2 only. Stage 1 still runs, and the RandomCrop/RandomSizedCrop inside Option B is what clips whole objects out of frame in the reported defect-detection setup.

How this PR fixes it

aug_config becomes the single knob that also governs the stage 1 crop branch. No new argument or config field is added.

aug_config Augmentations Training resize-and-crop branch
None (default) default AUG_CONFIG preset kept
{} none dropped, with a warning
non-empty dict the given transforms kept

None is unchanged from current behavior. The only behavior change is {}: it already meant "no augmentations", and it now also drops the resize-and-crop branch so training uses direct resize only. A warning is logged on {} so the change is not silent.

Implementation:

  • _crop_branch_enabled(aug_config) maps aug_config to a boolean and owns the {} warning.
  • _build_train_resize_config(..., include_crop_branch: bool) stays a pure config builder. When include_crop_branch=False it returns Option A only, dropping the resize, crop, resize branch.
  • make_coco_transforms and make_coco_transforms_square_div_64 derive include_crop_branch from aug_config via _crop_branch_enabled and forward it.

Fixes #1034

Usage

from rfdetr import RFDETRMedium

model = RFDETRMedium()

# Default: augmentations on, resize-and-crop branch on
model.train(dataset_dir="path/to/dataset")

# Disable augmentations AND the resize-and-crop branch (logs a warning)
model.train(dataset_dir="path/to/dataset", aug_config={})

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Testing

  • I have tested this change locally
  • I have added/updated tests for this change

Test details:

  • tests/datasets/test_coco.py::TestAugConfigDisablesCrop
    • test_crop_branch_enabled (parametrized over None / {} / non-empty) checks _crop_branch_enabled maps aug_config to the right decision.
    • test_empty_aug_config_warns checks aug_config={} logs a warning.
    • test_make_coco_transforms_forwards_crop_decision checks make_coco_transforms forwards the derived value to _build_train_resize_config.
  • tests/datasets/test_coco_resize_config.py::TestBuildTrainResizeConfigCropBranch::test_include_crop_branch_false_drops_crop_branch (parametrized over square / non-square) checks RandomCrop/RandomSizedCrop are absent when include_crop_branch=False.

Existing characterization tests in TestBuildTrainResizeConfigStructure cover the default kept-branch case.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors
  • [] I have updated the documentation accordingly (if applicable)

@codecov

codecov Bot commented May 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77%. Comparing base (680b586) to head (8e3d27a).

❌ Your project check has failed because the head coverage (77%) is below the target coverage (95%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #1037   +/-   ##
=======================================
  Coverage       77%     77%           
=======================================
  Files          102     102           
  Lines         9104    9111    +7     
=======================================
+ Hits          7037    7044    +7     
  Misses        2067    2067           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Borda Borda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see your point, but I'm not much in favor of adding a new argument. I would rather have aug_config=None to preserve actual structural behavior and None being the default, but enable forced reset with pasing aug_config={}.

@omkar-334

Copy link
Copy Markdown
Contributor Author

I see your point, but I'm not much in favor of adding a new argument. I would rather have aug_config=None to preserve actual structural behavior and None being the default, but enable forced reset with pasing aug_config={}.

Do you mean if aug_config is None, then _build_train_resize_config uses that and disables the resize&crop? basicaly reusing the same parameters?
or do you not want this functionality at all

@Borda

Borda commented May 14, 2026

Copy link
Copy Markdown
Member

Do you mean if aug_config is None, then _build_train_resize_config uses that and disables the resize&crop? basicaly reusing the same parameters?

Yes 🦝, but if you have another suggestion, happy to hear it...

@omkar-334

omkar-334 commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

I originally thought of that approach itself, but there are 2 things we should consider

  1. It merges two pipeline stages (resize-time geometry vs. augmentation-time pixel ops) into one knob. A user who wants {} augs but keeps the multi-scale resize+crop now has no way to express that.
  2. Changing what aug_config={} can be a behavior break for anyone currently passing {} and relying on crop staying on.

what do you think?

@Borda

Borda commented May 15, 2026

Copy link
Copy Markdown
Member
  • It merges two pipeline stages (resize-time geometry vs. augmentation-time pixel ops) into one knob. A user who wants {} augs but keeps the multi-scale resize+crop now has no way to express that.

We may move these multi-scale resize+crop to predefined augmentation, but need to check how to enforce/select which would use the receptive field size... so maybe really have these in predefined pipelines and in the model just add scaling to the model size as mandatory to not break the pre-processing

  • Changing what aug_config={} can be a behavior break for anyone currently passing {} and relying on crop staying on.

we can add some warning for people passing {}, which is use choce, not a default

cc: @isaacrob

omkar-334 added 2 commits May 19, 2026 08:59
Address PR review: replace the new do_random_crop argument with
aug_config-driven behavior, so no public knob is added.

- aug_config=None (default) and non-empty configs keep the training
  resize-and-crop branch; aug_config={} drops it, with a warning.
- Remove the do_random_crop config field and the make_coco_transforms
  parameters; add _resolve_do_random_crop() to derive the value from
  aug_config.
- _build_train_resize_config keeps its internal do_random_crop param.
- Update tests and the aug_config docstring.
@omkar-334

Copy link
Copy Markdown
Contributor Author

@Borda I removed the do_random_crop argument; aug_config now controls the resize-crop branch.

aug_config Augmentations Train resize-crop branch
None (default) default AUG_CONFIG preset kept
{} none dropped + warning
non-empty dict the given transforms kept

None is unchanged from current behavior. The only behavior change is {}: it already meant "no augmentations", and it now also drops the training resize-crop branch (direct resize only). A warning is logged on {} so that drop is not silent.

@omkar-334 omkar-334 changed the title add do_random_crop option to disable training resize-and-crop branch disable training resize-and-crop branch when aug_config={} May 19, 2026
@omkar-334 omkar-334 changed the title disable training resize-and-crop branch when aug_config={} warn and disable training resize-and-crop branch when aug_config={} May 19, 2026
@Borda Borda requested review from Borda and Copilot May 19, 2026 12:00
@Borda

Borda commented May 19, 2026

Copy link
Copy Markdown
Member

I removed the do_random_crop argument

I still see it...

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the COCO training transform pipeline so that passing aug_config={} not only disables augmentation stage transforms, but also disables the resize → (random) crop → resize branch in the training resize stage (with an explicit warning), addressing cases where cropping can remove objects/defects from frame.

Changes:

  • Added do_random_crop: bool = True to _build_train_resize_config(...) and made it return only the direct-resize branch when disabled.
  • Introduced _resolve_do_random_crop(aug_config) and used it in make_coco_transforms* to disable the crop branch (and warn) when aug_config == {}.
  • Added unit tests to validate the crop-branch removal and the forwarding of the derived decision.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/rfdetr/datasets/coco.py Adds do_random_crop support to the resize config builder and disables the crop branch when aug_config={} (with warning).
src/rfdetr/datasets/aug_config.py Updates the aug_config={} example comment to reflect the new behavior.
tests/datasets/test_coco.py Adds tests for _resolve_do_random_crop behavior, warning emission, and forwarding into _build_train_resize_config.
tests/datasets/test_coco_resize_config.py Adds a test ensuring the crop transforms are absent when do_random_crop=False.

Comment thread src/rfdetr/datasets/coco.py
Comment thread src/rfdetr/datasets/aug_config.py Outdated
Comment thread tests/datasets/test_coco.py Outdated
@omkar-334

Copy link
Copy Markdown
Contributor Author

I removed the do_random_crop argument

I still see it...

I removed the public-facing parameter. We still need an internal flag for enabling this right... should i rename it?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment thread src/rfdetr/datasets/coco.py Outdated
Comment on lines +407 to +412
if aug_config == {}:
logger.warning(
"aug_config={} disables the training resize-and-crop branch in addition to all "
"augmentations; images will not be randomly cropped. Pass aug_config=None to keep "
"the default resize pipeline."
)
@omkar-334

Copy link
Copy Markdown
Contributor Author

@Borda what do you think about the changes i made

@Borda

Borda commented May 26, 2026

Copy link
Copy Markdown
Member

Borda what do you think about the changes i made

a bit busy with next release, but will check when I have a moment... 🦝

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

src/rfdetr/datasets/coco.py:478

  • With aug_config={}, _crop_branch_enabled logs a warning and AlbumentationsWrapper.from_config(resolved_aug_config) will also emit the existing "Empty augmentation config provided" warning (because resolved_aug_config is {}). Consider skipping the from_config call (or otherwise suppressing that warning) when the resolved augmentation config is empty, so users only get the intentional new warning.
    if image_set == "train":
        resolved_aug_config = aug_config if aug_config is not None else AUG_CONFIG
        include_crop_branch = _crop_branch_enabled(aug_config)
        resize_wrappers = AlbumentationsWrapper.from_config(
            _build_train_resize_config(scales, square=False, max_size=1333, include_crop_branch=include_crop_branch)
        )
        pipeline = [*resize_wrappers]
        if not gpu_postprocess:
            aug_wrappers = AlbumentationsWrapper.from_config(resolved_aug_config)
            pipeline += [*aug_wrappers]

src/rfdetr/datasets/coco.py:566

  • Same as make_coco_transforms: with aug_config={}, _crop_branch_enabled logs a warning and AlbumentationsWrapper.from_config(resolved_aug_config) will also warn that the augmentation config is empty. Consider skipping the from_config call when resolved_aug_config is empty to avoid duplicate warnings.
    if image_set == "train":
        resolved_aug_config = aug_config if aug_config is not None else AUG_CONFIG
        include_crop_branch = _crop_branch_enabled(aug_config)
        resize_wrappers = AlbumentationsWrapper.from_config(
            _build_train_resize_config(scales, square=True, include_crop_branch=include_crop_branch)
        )
        pipeline = [*resize_wrappers]
        if not gpu_postprocess:
            aug_wrappers = AlbumentationsWrapper.from_config(resolved_aug_config)
            pipeline += [*aug_wrappers]

Comment on lines 469 to 474
if image_set == "train":
resolved_aug_config = aug_config if aug_config is not None else AUG_CONFIG
include_crop_branch = _crop_branch_enabled(aug_config)
resize_wrappers = AlbumentationsWrapper.from_config(
_build_train_resize_config(scales, square=False, max_size=1333)
_build_train_resize_config(scales, square=False, max_size=1333, include_crop_branch=include_crop_branch)
)
@Borda

Borda commented Jun 3, 2026

Copy link
Copy Markdown
Member

I removed the do_random_crop argument

I still see it...

I removed the public-facing parameter. We still need an internal flag for enabling this right... should i rename it?

Is the internal param/arg needed?

Co-authored-by: Codex <codex@openai.com>
Comment thread docs/learn/train/advanced.md Outdated
model.train(dataset_dir="path/to/dataset", aug_config={})
```

Passing `{}` also drops the training resize-and-crop branch, so images are resized

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lets format it as !!!Tip

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment on lines 292 to 298
def _build_train_resize_config(
scales: List[int],
*,
square: bool,
max_size: Optional[int] = None,
disable_augmentations: bool = False,
) -> List[Dict[str, Any]]:
Borda and others added 2 commits June 3, 2026 08:58
- Add `scale_jitter: bool = True` to `TrainConfig` — independent control for the resize→crop→resize branch (Option B) in the training pipeline; `aug_config` now governs the augmentation stack only
- Rename `disable_augmentations` → `disable_scale_jitter` in `_build_train_resize_config`; remove `_augmentations_disabled()` helper that coupled the two concerns
- Propagate `scale_jitter` through `build_coco`, `build_roboflow_from_coco`, `o365.py`, `yolo.py` (fixes previously unreachable O365 crop-disable path)
- Replace `TestAugConfigDisablesCrop` with `TestScaleJitter`; add square-variant forwarding test and regression asserting `aug_config={}` no longer affects the crop branch
- Delete `src/rfdetr/datasets/aug_config.py` (file renamed to `aug_configs.py` earlier); fix residual `aug_config` import paths in kornia_transforms, module_data, tests, notebooks
- Update docs (advanced.md, augmentations.md) to document `scale_jitter=False` as the independent crop-branch control

---
Co-authored-by: Claude Code <noreply@anthropic.com>
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.

Option to disable cropping and resizing during training

3 participants