-
Notifications
You must be signed in to change notification settings - Fork 27
add visual search #25
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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:
- 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)
- 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
The only thing missing here is to use the assigned V2 layer (found here model-tools/model_tools/brain_transformation/__init__.py Lines 51 to 56 in 2f778c6
|
self.target_model = target_model_param['target_model'] | ||
self.stimuli_model = stimuli_model_param['stimuli_model'] |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
No description provided.