-
Notifications
You must be signed in to change notification settings - Fork 0
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
_final_transformation
of FullyCNN not set during initialization
#63
Comments
I have a feeling the test is setting a placeholder value for gz21_ocean_momentum/src/gz21_ocean_momentum/trainScript.py Lines 299 to 314 in 5e5706e
It's a little clunky due to the dynamic module loading, which we could strip and add back only if required (it was not used). Maybe it belongs as an I guess setting the identity function as the default may be sensible...? But having it a required parameter would seem clearer. @tztsai what do you think? |
I agree that adding final_transformation to the argument list would be clearer, but it needs to be a callable object, so it cannot be directly specified from the CMD arguments. Perhaps a map from names to callable transformations could be added so that the user can provide a string as an argument and it will be mapped to a transformation, e.g. {'identity': lambda x: x}? |
Directly passing a tensor to
FullyCNN()
will causeAttributeError: 'FullyCNN' object has no attribute 'final_transformation'
. In the test of FullyCNN, the_final_transformation
attribute is manually set. Perhaps this line should be moved into the__init__
method of FullyCNN.Besides, there is a Mixin class related to the final transformation here, but the FullyCNN class does not inherit it. It instead implements the setter and getter methods of
final_transformation
in itself.The text was updated successfully, but these errors were encountered: