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 broken IJEPA Example #1712

Open
Tracked by #1320
MalteEbner opened this issue Oct 28, 2024 · 4 comments · May be fixed by #1747
Open
Tracked by #1320

Fix broken IJEPA Example #1712

MalteEbner opened this issue Oct 28, 2024 · 4 comments · May be fixed by #1747
Labels

Comments

@MalteEbner
Copy link
Contributor

MalteEbner commented Oct 28, 2024

When running python examples/pytorch/ijepa.py, the code fails:

  File "/Users/malteebnerlightly/Documents/GitHub/lightly/lightly/models/modules/ijepa.py", line 226, in forward
    input = utils.apply_masks(input, idx_keep)

The utils are imported as from lightly.models import utils.

The function lightly.models/utils.py:apply_masks() was added in a55cf44 and removed later.

@MalteEbner MalteEbner added the bug label Oct 28, 2024
@MalteEbner MalteEbner mentioned this issue Oct 28, 2024
10 tasks
@MalteEbner MalteEbner changed the title IJepa Example is broken Fix broken IJEPA Example Oct 28, 2024
@vectorvp
Copy link
Contributor

@MalteEbner, hello, so the idea is to return apply_masks() to utils.py?

@guarin
Copy link
Contributor

guarin commented Nov 26, 2024

Yes that would be a great start. I'm not sure if something else is broken as well. The relevant code from the original PR is:

def apply_masks(x, masks):
    """
    :param x: tensor of shape [B (batch-size), N (num-patches), D (feature-dim)]
    :param masks: list of tensors containing indices of patches in [N] to keep
    """
    all_x = []
    for m in masks:
        mask_keep = m.unsqueeze(-1).repeat(1, 1, x.size(-1))
        all_x += [torch.gather(x, dim=1, index=mask_keep)]
    return torch.cat(all_x, dim=0)

@vectorvp
Copy link
Contributor

@guarin, apply_masks together with repeat_interleave_batch were moved to lightly/models/modules/ijepa_timm.py. However, timm is used nowhere. Does this module make sense? Do we want to remove it and just add the functions we need to utils.py?

@vectorvp vectorvp linked a pull request Nov 29, 2024 that will close this issue
@vectorvp
Copy link
Contributor

I've delved into the conversations from past issues regarding this timm module and plans to move to it from the torchvision. Based on that I just moved apply_masks together with repeat_interleave_batch to utils.py from lightly/models/modules/ijepa_timm.py.

Now the example works. PR is attached.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants