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

add require_variance to model_interface look_at signature #652

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

benlonnqvist
Copy link
Contributor

@benlonnqvist benlonnqvist commented Mar 18, 2024

This PR resolves issue #651.

In addition, this PR adds traversal paths for require_variance for non-neural task models (Behavior, ProbabilitiesMapping, Temporal).

brainscore_vision/model_interface.py Outdated Show resolved Hide resolved
Co-authored-by: Martin Schrimpf <mschrimpf@users.noreply.github.com>
@deirdre-k
Copy link
Contributor

Hi @benlonnqvist, is this PR still in progress? If not, should it be closed? Thanks for the contributions!

@benlonnqvist
Copy link
Contributor Author

Hi @deirdre-k, thanks for the message. This PR is technically still in progress and while PR #917 fixed the issue about being able to call microsaccades, the signature issue in model_interface / elsewhere still seems to be open.

We could either:

  1. wait for me to get to this at some point in the future (I see this as quite a low priority item for me personally)
  2. close this and leave the issue require_variance missing from the signature of look_at #651 open with a note pointing to this PR as a somewhat-approved way of resolving the issue

Thanks!

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