-
Notifications
You must be signed in to change notification settings - Fork 21
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
Feature inversion pipeline for modular iCNN construction #81
Conversation
@ganow class BaseGenerator(ABC):
"""Generator module."""
@abstractmethod
def reset_states(self) -> None:
"""Reset the state of the generator."""
pass
@abstractmethod
def parameters(self, recurse: bool = True) -> Iterator[nn.Parameter]:
"""Return the parameters of the generator."""
pass
@abstractmethod
def generate(self, latent: torch.Tensor) -> torch.Tensor:
"""Forward pass through the generator network.
Parameters
----------
latent : torch.Tensor
Latent vector.
Returns
-------
torch.Tensor
Generated image. The generated images must be in the range [0, 1].
"""
def __call__(self, latent: torch.Tensor) -> torch.Tensor:
"""Call self.generate().
Parameters
----------
latent : torch.Tensor
Latent vector.
Returns
-------
torch.Tensor
Generated image. The generated images must be in the range [0, 1].
"""
return self.generate(latent) 意図は、メソッドに名前をあたえることでそれぞれのメソッドの役割を明確化することです。あとこれはほとんど私情ですが、親クラスのプライベートメソッドをオーバーライドすることに気持ち悪さがあります。 |
なるほど。実装するメソッド名はモジュールごとに変えずにforwardで共通化するのはどう思いますか?nn.moduleと透過的に扱うことを意識したAPIです |
sample codeを作成してのコメント
|
Co-authored-by: Yoshihiro Nagano <[email protected]>
Co-authored-by: Yoshihiro Nagano <[email protected]>
Co-authored-by: Yoshihiro Nagano <[email protected]>
Co-authored-by: Yoshihiro Nagano <[email protected]>
Co-authored-by: Yoshihiro Nagano <[email protected]>
Co-authored-by: Yoshihiro Nagano <[email protected]>
Co-authored-by: Yoshihiro Nagano <[email protected]>
Feature inversion task (#81) に関わるPR内のテストコードの作成
self._callback_handler = CallbackHandler(callbacks) | ||
|
||
@abstractmethod | ||
def __call__(self, *inputs, **parameters) -> Any: |
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.
Instead of directly overriding __call__
, I prefer the implementation of having an abstract method (e.g., run
) and calling it in __call__
.
self._callback_handler.fire("on_task_end") | ||
return generated_image | ||
|
||
def reset_states(self) -> None: |
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.
Could you remind me of the reason why reset_states
is not called in the beginning of __call__
(i.e., why users need to explicitly call reset_states
)?
bdpy/recon/torch/modules/critic.py
Outdated
return self.compare(features, target_features) | ||
|
||
@abstractmethod | ||
def compare( |
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.
The name "comapre" sounds a bit ambiguous (but I don't have any alternative yet).
bdpy/recon/torch/task/inversion.py
Outdated
|
||
self._num_iterations = num_iterations | ||
|
||
def __call__( |
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.
See comments on bdpy/task/core.py
#77
TODO:
(I will work on this after Refactor test modules #66 is merged on the dev branch)dataset