Skip to content

[WIP] small changes for methods.gradient_ascent - #7

Open
KonstantinWilleke wants to merge 31 commits into
sinzlab:masterfrom
KonstantinWilleke:group_mei_plaground
Open

[WIP] small changes for methods.gradient_ascent#7
KonstantinWilleke wants to merge 31 commits into
sinzlab:masterfrom
KonstantinWilleke:group_mei_plaground

Conversation

@KonstantinWilleke

Copy link
Copy Markdown
Contributor

No description provided.

@christoph-blessing christoph-blessing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you please run black over this and check if all of the tests still pass?

Comment thread mei/modules.py Outdated
"""

def __init__(self, model: Module, constraint: int, forward_kwargs: Dict[str, Any] = None):
def __init__(self, model: Module, constraint: int, target_fn=None, forward_kwargs: Dict[str, Any] = None):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a type annotation for the new argument.

Comment thread mei/modules.py
model: A PyTorch module.
constraint: An integer representing the index of a neuron in the model's output. Only the value corresponding
to that index will be returned.
target_fn: Callable, that gets as an input the constrained output of the model.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this new argument needed?

Comment thread mei/modules.py Outdated
"""
output = self.model(x, *args, **self.forward_kwargs, **kwargs)
return output[:, self.constraint]
return self.target_fn(output[:, self.constraint])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should be tested if the model output gets passed to the function and if the return value of the function gets returned.

Comment thread mei/initial.py


class RandomNormalNullChannel(InitialGuessCreator):
"""Used to create an initial guess tensor filled with values distributed according to a normal distribution."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This docstring is the same as the one for the RandomNormal initial guess creator. This might confuse people as to what the differences between the two are.

Comment thread mei/initial.py

_create_random_tensor = randn

def __init__(self, null_channel, null_value=0):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would like to see type annotations here.

Comment thread mei/initial.py
self.null_value = null_value

def __call__(self, *shape):
"""Creates a random initial guess from which to start the MEI optimization process given a shape."""

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is also the same docstring as the one of the __call__ method of the RandomNormal` initial guess creator.

Comment thread mei/initial.py Outdated
return inital

def __repr__(self):
return f"{self.__class__.__qualname__}()" No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The file is missing a newline at the end

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.

2 participants