-
Notifications
You must be signed in to change notification settings - Fork 13
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
updated for keras 2.2+, tf2 #12
Conversation
I've added some very basic tests here, modernized the package metadata/installation, and set up github actions for CI testing and publishing. The actions should take effect when this merges. I'll hold off on a torch implementation for a separate PR, as that will require a bit more thought about API. Otherwise I think this is good to merge. |
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.
lgtm, see minor comments about docs and docstrings
python -m pip install autopool[keras] | ||
``` | ||
For the tensorflow implementation: | ||
``` | ||
python -m pip install autopool[tf] |
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.
What happens if you call pip install autopool
withtout the keras/tf specification? Is there a default behavior?
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.
It installs with no dependencies. We actually have no (runtime) dependencies beyond the python core until getting into specific framework implementations.
pip install -u autopool | ||
|
||
to install just for your own user. | ||
python -m pip install autopool[tf] |
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.
cf prev comment, what happens if pip install autopool
?
|
||
|
||
class AutoPool1D(Layer): | ||
'''Automatically tuned soft-max pooling. (tensorflow.keras implementation) |
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.
tensorflow.keras implementation?
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.
Yes, as opposed to the keras implementation, or a low-level tensorflow implementation (Does not exist)
This PR updates autopool to work with keras >= 2.2 and/or tensorflow 2.0
I'm also rolling in some restructuring as noted in #13 and some general package maintenance as needed.