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

kwcoco split --rng 12345 does not split different files referring to same videos uniformly #6

Open
cameron-a-johnson opened this issue Oct 31, 2024 · 1 comment

Comments

@cameron-a-johnson
Copy link

Hey! Paul might share this too: a pipeline produces multiple kwcoco datasets for the same set of videos, with frame-wise activity classification ground truth, pose detection, and some object detections, respectively.

FWIW, we want to kwcoco split each of those by the same random split. I thought kwcoco split --rng 12345 might create the same split on each file but it doesn't.

And that's really no big deal for us! Obviously a ~5-line splitter script that seeds a random split of the video id list will do.

Just thought the use case might be interesting to consider. If so, maybe I could take a stab at a re-implementation of split that would work for this case?

@Erotemic
Copy link
Member

Erotemic commented Nov 1, 2024

I think only a minor modification is needed.

When balance_categories is True the current behavior is correct because the splits depend on the annotations. If you have two datasets with the same video AND annotation distribution you would expect they have the same split, but otherwise it may vary.

However, when balance_categories is False, then a consistent split over video-ids would make much more sense. The problem logic is here:

        # setup
        dset = kwcoco.CocoDataset.coerce(config['src'])
        images = dset.images()
        cids_per_image = images.annots.cids
        gids = images.lookup('id')
        group_ids = images.lookup('video_id')
        ...

        final_group_ids = []
        final_group_gids = []
        final_group_cids = []

        unique_cids = set(ub.flatten(cids_per_image)) | {0}
        distinct_cid = max(unique_cids) + 11

        for group_id, gid, cids in zip(group_ids, gids, cids_per_image):
            if len(cids) == 0:
                final_group_ids.append(group_id)
                final_group_gids.append(gid)
                final_group_cids.append(distinct_cid)
            else:
                final_group_ids.extend([group_id] * len(cids))
                final_group_gids.extend([gid] * len(cids))
                final_group_cids.extend(cids)

        # Balanced category split
        rng = kwarray.ensure_rng(config['rng'])

        shuffle = rng is not None
        factor = config['factor']
        self = util_sklearn.StratifiedGroupKFold(n_splits=factor,
                                                 random_state=rng,
                                                 shuffle=shuffle)

        if config['balance_categories']:
            split_idxs = list(self.split(X=final_group_gids, y=final_group_cids, groups=final_group_ids))
        else:
            split_idxs = list(self.split(X=final_group_gids, y=final_group_gids, groups=final_group_ids))

if not config['balance_categories'], then we should not need to duplicate image-ids for the sklearn splitter. I think you can just take the len(cids) == 0 path in this case, and it should work.

Of course, we should also write a test for this as well.

Even though most development of kwcoco is on gitlab, I think it makes sense to start using github more, as it should encourage more community contributions. Towards this end, I should enable CI on github for this package. So, please submit a PR here if you can help fix this.

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

No branches or pull requests

2 participants