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

Access to intermediate embeddings #520

Closed
wants to merge 2 commits into from
Closed

Access to intermediate embeddings #520

wants to merge 2 commits into from

Conversation

blazejba
Copy link
Collaborator

This change allows access to intermediate embeddings from the model. Example of usage:

output, extras = model(batch, extra_return_names=["pre_task_heads"])
fingerprint_graph = extras['pre_task_heads']['graph_feat']

@blazejba blazejba requested a review from DomInvivo as a code owner July 17, 2024 11:55
@DomInvivo DomInvivo requested a review from WenkelF July 17, 2024 21:09
Copy link
Collaborator

@DomInvivo DomInvivo left a comment

Choose a reason for hiding this comment

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

It doesn't seem to break anything, so fine by me, but @WenkelF would likely be better suited to approve it.

@WenkelF
Copy link
Collaborator

WenkelF commented Jul 17, 2024

I agree with @DomInvivo that the changes don't break anything (although I see the unit tests failing).

Out of curiosity, what is the purpose of the changes @blazejba? It looks like they make the input of the task heads accessible as fingerprints when running a forward pass.

We also have this logic for fingerprinting. It allows not only to read out the inputs of the task heads but any intermediate representation of the model. Maybe there is a way of consolidating here if it serves the same purpose to avoid redundancies. But I am also ok with the changes as is.

@blazejba
Copy link
Collaborator Author

@WenkelF - Thanks for the pointer. I wasn't aware it's already implemented - it's exactly what I've been looking for. I am closing this PR as it is no longer necessary.

@blazejba blazejba closed this Jul 18, 2024
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.

3 participants