-
Notifications
You must be signed in to change notification settings - Fork 61
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
adding version checks for backends #412
Conversation
Codecov Report
@@ Coverage Diff @@
## bkdocs #412 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 79 79
Lines 11398 11399 +1
=========================================
+ Hits 11398 11399 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Thanks, the check looks good but only checks the tensorflow version. If I understood correctly your comment here we would also like to check that the qibotf version is compatible with the qibo version.
@@ -2,6 +2,9 @@ | |||
from qibo import config | |||
from qibo.config import raise_error, log, warnings | |||
|
|||
# versions requirements | |||
TF_MIN_VERSION = '2.2.0' |
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.
We could also move this to config.py.
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.
Any special reason to move that to config.py
?
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.
No, other than that we define all other "constants" used throughout the qibo code there. I don't have any strong preference on what is the right position for TF_MIN_VERSION
though.
I believe the only strong requirement we have now is the tf version, given that for qibotf we don't have any specific API that requires locking their version. |
I agree with this, so I guess this should be fine to merge now and if at some point we create some API inconsistency between qibo and qibotf we can use the same template the check the corresponding versions. |
@stavros11, yes, thanks, I agree. |
Implements a simple version check for backends, including the mirroring with setup.py.