Closed dimidagd closed 2 weeks ago
The recent update to the fiftyone/utils/torch.py
file allows the _build_transforms
method to dynamically set the ragged_batches
parameter based on the provided config.transforms_args
configuration. This replaces the previous hardcoded value of True
for ragged_batches
, enhancing flexibility when handling batch processing in Torch-based models.
File | Change Summary |
---|---|
fiftyone/utils/torch.py |
Updated _build_transforms method to use config.transforms_args.get("ragged_batches", True) instead of hardcoded True for ragged_batches . |
No sequence diagrams are provided because the changes are too focused and straightforward to benefit from visual representation.
ragged_batches
and the inability to run batched predictions. The current changes address this by allowing the ragged_batches
parameter to be configured dynamically.In code where batches used to lag, Now dynamic, no longer a drag. Configs now steer, Transformations clear, Our models march forward, no sag. ππ
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
All modified and coverable lines are covered by tests :white_check_mark:
Please upload report for BASE (
fix/ragged-batches@5ced55e
). Learn more about missing BASE report.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@dimidagd thanks for the PR! I see the issue and agree that users should have the ability to explicitly configure
ragged_batches
when using custom transforms.I think it would be best to add a new (optional)
ragged_batches
parameter rather than pulling it fromtransforms_args["ragged_batches"]
, as the latter is supposed to be usable in conjunction withtransforms_fcn
like this:transforms = transforms_fcn(**transforms_args)
I'll go ahead and merge this PR into a feature branch so you get credit for your contribution(!) and will follow up with a commit that migrates the new parameter per the above.
This requires changes in other parts of the codebase as I've understood.
This requires changes in other parts of the codebase as I've understood.
This is all that's required: https://github.com/voxel51/fiftyone/pull/4512
What changes are proposed in this pull request?
Resolves https://github.com/voxel51/fiftyone/issues/4508
(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyone
Python library changesSummary by CodeRabbit