Skip to content
This repository was archived by the owner on Jan 5, 2024. It is now read-only.

Conversation

@shashikg
Copy link

No description provided.

Copy link
Member

@mschrimpf mschrimpf left a comment

Choose a reason for hiding this comment

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

there are some dependencies that I think shouldn't be here:

  1. model-tools (this repo) should not depend on candidate_models. Instead, models should be passed externally in the init function (check e.g. here for an example of what I mean)
  2. some pieces of the code seem to be specific to the data that is used in the specific benchmark. Ideally this code would be independent of the specific data so that it can be applied to other data too, but I understand if that's too much work

It would also be great if there was a unit-test to demonstrate and test the functionality added here

@mschrimpf
Copy link
Member

The only thing missing here is to use the assigned V2 layer (found here

def commit_region(self, region):
layer_selection = LayerSelection(model_identifier=self.layer_model.identifier,
activations_model=self.layer_model.activations_model, layers=self.layers)
benchmark = self.region_benchmarks[region]
best_layer = layer_selection(selection_identifier=region, benchmark=benchmark)
self.layer_model.commit(region, best_layer)
) for the search image features, right?

@shashikg
Copy link
Author

@mschrimpf

Comment on lines +166 to +167
self.target_model = target_model_param['target_model']
self.stimuli_model = stimuli_model_param['stimuli_model']
Copy link
Member

Choose a reason for hiding this comment

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

should these not be the same? Sorry I'm getting confused with these parameters hidden in dictionaries

Copy link
Author

Choose a reason for hiding this comment

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

No these will not be the same. As I previously mentioned that the input image size will be different for the target_image and stimuli_image. You will need two different ML models for both.

def start_task(self, task: BrainModel.Task, **kwargs):
self.fix = kwargs['fix'] # fixation map
self.max_fix = kwargs['max_fix'] # maximum allowed fixation excluding the very first fixation
self.data_len = kwargs['data_len'] # Number of stimuli
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this just be the length of the stimulus_set, why do we need to pass this?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, this will be the same. I will change this. Thanks for pointing out.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants