Skip to content

Conversation

@lucaeyring
Copy link
Collaborator

This PR adds the functionality to save multiple intermediate features in a single forward pass.

This is implemented through a new argument module_names. Now either module_name or module_names need to be supplied. This is done for backwards compatibility (everything should still work with just module_name).

One change needed to be made, which is that features is now a Dict from module_name to corresponding features_module, and the filename, which the features are saved under, now includes the module_name. Need to double check whether this breaks anything w.r.t. backward compatibility.

@lciernik lciernik requested review from LukasMut and lciernik July 29, 2025 12:10
@LukasMut LukasMut requested a review from MarcoMorik July 31, 2025 11:52
@LukasMut LukasMut added the enhancement New feature or request label Jul 31, 2025
@LukasMut LukasMut added the cleanup Deprecate or refactor code label Jul 31, 2025
Copy link
Collaborator

@lciernik lciernik left a comment

Choose a reason for hiding this comment

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

I left some comments on where things could be done slightly differently.

Some questions remain:

  • Where is the backward compatibility logic when module_name is given? It appears to me that _extract_batch will, in any case, return a dictionary of module names and features.
  • We can apply alignment of representations. See align method in torch.py. Does this still work with multiple module_names?
  • What happens if the dataset is huge?

Copy link
Collaborator

@lciernik lciernik left a comment

Choose a reason for hiding this comment

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

Still have some comments.

@LukasMut LukasMut added this pull request to the merge queue Aug 6, 2025
Merged via the queue into ViCCo-Group:master with commit 027f3c0 Aug 6, 2025
1 of 3 checks passed
@LukasMut
Copy link
Collaborator

LukasMut commented Aug 6, 2025

This fixes issue #89

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Deprecate or refactor code enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants