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

Multiple crops example #5656

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

mzient
Copy link
Contributor

@mzient mzient commented Oct 2, 2024

Category:

Other (e.g. Documentation, Tests, Configuration)

Description:

Implements a pipeline example requested by a user in #1579

Additional information:

Affected modules and functionalities:

N/A

Key points relevant for the review:

Tests:

  • Existing tests apply
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: N/A

Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [18958041]: BUILD STARTED

Signed-off-by: Michał Zientkiewicz <mzient@gmail.com>
@mzient mzient force-pushed the multiple_crops_example branch from 614b649 to fc05029 Compare October 2, 2024 14:55
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [18958465]: BUILD STARTED

@NVIDIA NVIDIA deleted a comment from dali-automaton Oct 2, 2024
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [18958465]: BUILD PASSED

@stiepan
Copy link
Member

stiepan commented Oct 4, 2024

For the future, I belive we could make the resize and crop ops support similar trick, likely in a bit more straightforward way.

Copy link

review-notebook-app bot commented Oct 4, 2024

View / edit / reply to this conversation on ReviewNB

szkarpinski commented on 2024-10-04T11:44:12Z
----------------------------------------------------------------

Line #42.        )  # convert to float and mark outermost dimension as"frame"

Nitpicks: Why +0.0 and not cast? Also, there's a missing space before "frame"


Copy link

review-notebook-app bot commented Oct 4, 2024

View / edit / reply to this conversation on ReviewNB

szkarpinski commented on 2024-10-04T11:44:13Z
----------------------------------------------------------------

How about adding a sentence explaining what the images above mean? The first single image was confusing to me - I expected sequences of equal length. It took me some time to notice num_crops = random (...) in the code.


mzient commented on 2024-11-04T11:46:51Z
----------------------------------------------------------------

Yeah, I'd recommend playing with the seed a bit to get an example with three different sequence lengths, preferably without a single-image sequence.

Copy link
Contributor Author

mzient commented Nov 4, 2024

Yeah, I'd recommend playing with the seed a bit to get an example with three different sequence lengths, preferably without a single-image sequence.


View entire conversation on ReviewNB

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.

4 participants