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

fix tests, small optimization to json dataset loading #241

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

Conversation

neph1
Copy link

@neph1 neph1 commented Jan 23, 2025

This fixes the dataset tests that were referencing the old cogvideox repo.
I also noticed while doing so that using ImageOrVideoDataset directly will throw an exception because max_num_frames is not set. So I moved that to init of the base class.

There's also a minor refactor to the _load_dataset_from_json method, because I initially read it wrong and thought it was parsing two columns separately. At least it only parses the data once.

I still haven't done any test run with this. So maybe let it simmer for a bit.

Ref: #239

Comment on lines +144 to +147
try:
video = self._preprocess_video(video_path)
except:
raise ValueError(f"Exception while preprocessing {video_path}.")
Copy link
Author

Choose a reason for hiding this comment

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

This was added for personal reasons, as I had difficulties finding out which file was causing issues

@@ -249,8 +258,6 @@ class ImageOrVideoDatasetWithResizing(ImageOrVideoDataset):
def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)

self.max_num_frames = max(self.resolution_buckets, key=lambda x: x[0])[0]
Copy link
Author

Choose a reason for hiding this comment

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

Moved to ImageOrVideoDataset

finetrainers/dataset.py Outdated Show resolved Hide resolved
@neph1 neph1 marked this pull request as draft January 23, 2025 20:18
@neph1 neph1 marked this pull request as ready for review January 27, 2025 20:36
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.

1 participant