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

Add FlyingThings3D dataset for optical flow #4858

Merged
merged 20 commits into from
Nov 5, 2021
Merged

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Nov 4, 2021

This PRs adds the FlyingThings3D dataset for optical flow

cc @pmeier

@facebook-github-bot
Copy link

facebook-github-bot commented Nov 4, 2021

💊 CI failures summary and remediations

As of commit ba4cfd6 (more details on the Dr. CI page):


  • 1/1 failures introduced in this PR

1 failure not recognized by patterns:

Job Step Action
CircleCI binary_libtorchvision_ops_android Build 🔁 rerun

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@NicolasHug NicolasHug marked this pull request as ready for review November 4, 2021 16:31
Copy link
Collaborator

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Some minor comments inline plus you can apply the same comments I had in #4860 here. Otherwise LGTM, thanks @NicolasHug!

Comment on lines 2019 to 2024
for pass_name in passes:
for split in splits:
for letter in letters:
for subfolder in subfolders:
current_folder = root / pass_name / split / letter / subfolder
for camera in cameras:
Copy link
Collaborator

Choose a reason for hiding this comment

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

That looks kind of horrifying. Maybe itertools.product?

Comment on lines 2033 to 2043
for split in splits:
for letter in letters:
for subfolder in subfolders:
for direction in directions:
current_folder = root / "optical_flow" / split / letter / subfolder / direction
for camera in cameras:
os.makedirs(str(current_folder / camera))
for i in range(num_images_per_camera):
datasets_utils.make_fake_pfm_file(
self.FLOW_H, self.FLOW_W, file_name=str(current_folder / camera / f"{i}.pfm")
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here.

Comment on lines 2060 to 2063
def test_bad_input(self):
with pytest.raises(ValueError, match="split must be either"):
with self.create_dataset(split="bad"):
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you go with my suggestion from #4860 (comment), we probably don't need these tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll go with your suggestion but still keep the test 🤓

FlyingThings3D

Args:
root (string): Root directory of the Sintel Dataset.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
root (string): Root directory of the Sintel Dataset.
root (string): Root directory of the FlyingThings3D Dataset.

torchvision/datasets/_optical_flow.py Outdated Show resolved Hide resolved
@NicolasHug NicolasHug merged commit 1bd131c into main Nov 5, 2021
@github-actions
Copy link

github-actions bot commented Nov 5, 2021

Hey @NicolasHug!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py

facebook-github-bot pushed a commit that referenced this pull request Nov 8, 2021
Reviewed By: kazhang

Differential Revision: D32216678

fbshipit-source-id: 1e173234e775e3b6dff2a24e0f257b1d9176eb5c
cyyever pushed a commit to cyyever/vision that referenced this pull request Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants