-
Notifications
You must be signed in to change notification settings - Fork 129
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
[MNT, DOC] Accelerating deep testing #1904
base: main
Are you sure you want to change the base?
Changes from 9 commits
d138dda
bead7e3
f0e2f86
c29da6a
2eb10a3
dc6c597
c32ecf1
f982e24
8e582c5
4050ae3
10a270b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,36 +138,43 @@ def build_network(self, input_shape, **kwargs): | |
self._kernel_size_ = [8, 5, 3] if self.kernel_size is None else self.kernel_size | ||
|
||
if isinstance(self._n_filters_, list): | ||
assert len(self._n_filters_) == self.n_residual_blocks | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. raise an actual error with a message instead of asserting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will do, we should raise an issue to do that all over the networks module, my code my bad ! never thought about raising a message There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better as a |
||
self._n_filters = self._n_filters_ | ||
else: | ||
self._n_filters = [self._n_filters_] * self.n_residual_blocks | ||
|
||
if isinstance(self._kernel_size_, list): | ||
assert len(self._kernel_size_) == self.n_conv_per_residual_block | ||
self._kernel_size = self._kernel_size_ | ||
else: | ||
self._kernel_size = [self._kernel_size_] * self.n_conv_per_residual_block | ||
|
||
if isinstance(self.strides, list): | ||
assert len(self.strides) == self.n_conv_per_residual_block | ||
self._strides = self.strides | ||
else: | ||
self._strides = [self.strides] * self.n_conv_per_residual_block | ||
|
||
if isinstance(self.dilation_rate, list): | ||
assert len(self.dilation_rate) == self.n_conv_per_residual_block | ||
self._dilation_rate = self.dilation_rate | ||
else: | ||
self._dilation_rate = [self.dilation_rate] * self.n_conv_per_residual_block | ||
|
||
if isinstance(self.padding, list): | ||
assert len(self.padding) == self.n_conv_per_residual_block | ||
self._padding = self.padding | ||
else: | ||
self._padding = [self.padding] * self.n_conv_per_residual_block | ||
|
||
if isinstance(self.activation, list): | ||
assert len(self.activation) == self.n_conv_per_residual_block | ||
self._activation = self.activation | ||
else: | ||
self._activation = [self.activation] * self.n_conv_per_residual_block | ||
|
||
if isinstance(self.use_bias, list): | ||
assert len(self.use_bias) == self.n_conv_per_residual_block | ||
self._use_bias = self.use_bias | ||
else: | ||
self._use_bias = [self.use_bias] * self.n_conv_per_residual_block | ||
|
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.
Can this not just accept any
BaseClusterer
? Creating a useless option solely for testing is not a great way to resolve this IMO.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.
i wanted to change that for accepting an estimator input instead of string, but thought it might be a lot for the PR, but to keep the PR for testing purpose this can be done, if you think its ok to get all in one PR i dont mind can do the changes here
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.
Don't mind if you do it here. The dummy option is not a good addition IMO.
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.
will add the changes then