Activations #708
Activations #708
Conversation
There was a problem hiding this comment.
Don't mix different entities in code: each class/module should be responsible for its own thing. Rethink architecture of the proposed changes and their location in our code. One example of this can be:
- getting names / layers of a
TorchModel/Networkcan be a part of the model files, maybe even as a method of it. In that case, it should be automatic and not hardcoded; - an utility function to reduce the number of channels can be placed near to the model. Extra thought: you can do PCA on GPU: in that case, it can be integrated to the model
outputsparameter smoothly and would probably look beautiful; - any batch-related things look absolutely random here and do not belong. Do we even need them?
batchflow/models/torch/utils.py
Outdated
| @@ -118,3 +120,48 @@ def center_crop(inputs, target_shape, dims): | |||
| crops_ = [slice(crop_lefts[i], crop_lefts[i] + target_shape[i]) for i in range(dims)] | |||
| crops = [slice(dim) for dim in no_crop_shape] + crops_ | |||
| return inputs[crops] | |||
|
|
|||
|
|
|||
| def get_blocks_and_activations(model): | |||
There was a problem hiding this comment.
Too much hardcode here: if this code becomes a part of TorchModel, it should utilize and use understanding of classes involved. As of now, this code works only for a selected subclass of models.
As model (class Network) consists of pre-defined modules (EncoderModule, DecoderModule, DefaultModule, etc), you can check the instance of submodules and extract necessary names.
batchflow/models/torch/utils.py
Outdated
|
|
||
| return blocks, activation_names | ||
|
|
||
| def compress_activations(batch, activation_names, **kwargs): |
There was a problem hiding this comment.
batch has no relation to TorchModel and will not be part of the model-related-utils
batchflow/models/torch/utils.py
Outdated
|
|
||
| return batch, explained_variance | ||
|
|
||
| def reduce_channels(images, n_components=3, **kwargs): |
There was a problem hiding this comment.
Would like an option to apply a user-provided PCA instance (so that we can accumulate data in it). Also, normalization should be optional
batchflow/models/torch/base.py
Outdated
| activation_blocks.extend(blocks) | ||
| activation_names += [f'{module}_{i}' for i in range(len(blocks))] | ||
|
|
||
| return activation_blocks, activation_names |
There was a problem hiding this comment.
Do you even need activation_names?
There was a problem hiding this comment.
Do you even need
activation_names?
I use them to assign activations to batch, as well as to provide titles for plots.
There was a problem hiding this comment.
Well, it is how you use them. Why something related to your code (batch and plots), that don't belong here, should be there?
batchflow/models/torch/base.py
Outdated
| activation_blocks.extend(blocks) | ||
| activation_names += [f'{module}_{i}' for i in range(len(blocks))] | ||
|
|
||
| return activation_blocks, activation_names |
There was a problem hiding this comment.
Well, it is how you use them. Why something related to your code (batch and plots), that don't belong here, should be there?
No description provided.