Pin onnx version #3003
Pin onnx version #3003
Conversation
setup.py
Outdated
| @@ -223,7 +223,7 @@ def run(self): | |||
| }, | |||
| py_modules=python_modules_list, | |||
| install_requires=[ | |||
| 'onnx>=1.2.3', | |||
| 'onnx>=1.6.0,<1.7.0', | |||
| 'numpy>=1.18.0' | |||
There was a problem hiding this comment.
- Does onnx guarantee backward compatibility for versions between 1.6.0 and 1.7.0?
- Should we pin numpy as well?
There was a problem hiding this comment.
- Does onnx guarantee backward compatibility for versions between 1.6.0 and 1.7.0?
- Should we pin numpy as well?
btw, why are we requiring numpy 1.18.0 ? it was only released on Dec 22, 2019.
I don't think many users would like requiring such new software. and we don't even use numpy 1.18 specific api's in ORT.
There was a problem hiding this comment.
Because ort is built with that version, and numpy equal or older than 1.16 has security issues. We can change it to 1.17 but it doesn't make too much difference
There was a problem hiding this comment.
Because ort is built with that version, and numpy equal or older than 1.16 has security issues. We can change it to 1.17 but it doesn't make too much difference
yes, older than 1.16, that means the user could use 1.16, 1.17 or 1.18
ideally we should be using the lowest version possible to maximize numpy compatibility.
There was a problem hiding this comment.
I think it's fine. Numpy is always backward compatible (as long as the major version didn't change), so it won't cause any compatibility issue to the end users. They just get the lastest version and use it.
There was a problem hiding this comment.
I think it's fine. Numpy is always backward compatible (as long as the major version didn't change), so it won't cause any compatibility issue to the end users. They just get the lastest version and use it.
users do not like (and may not be able) to change stable production environments. software version updates can have other unintended downstream impacts.
numpy doesn't actually guarantee backwards compatibility for minor version updates. (it doesn't follow semver)
1.18 does have various api changes/deprecations. so i don't think it's safe to assume that users have zero impact for updates.
it's highly unusual to require users to update a dependency to the latest version that was only released a little over a month ago.
i suspect we will get complaints if we do this. if numpy 1.16 is secure and supported, we shouldn't move to 1.18 without good reason. especially as we are not using any specific features in 1.18
There was a problem hiding this comment.
The security team says: "An issue was discovered in NumPy 1.16.0 and earlier." So 1.16 isn't an option.
There was a problem hiding this comment.
The security team says: "An issue was discovered in NumPy 1.16.0 and earlier." So 1.16 isn't an option.
do you know which security issue it is? i.e. which 1.16.x works? I assume they would have backported security fixes to 1.16.x release?
note that 1.16.6 was released Dec 29, 2019 and is the most current 1.16.x release.
There was a problem hiding this comment.
There was a problem hiding this comment.
Talked offline. We definitely can try it, but I suggest spliting the change into a different PR, because it requires to change a lot of files.
|
@jywu-msft what was the final decision around this? would be good to include it in 1.2 release. |
The conclusion was to use the lowest numpy version possible, no? |
|
Close it to avoid doing unnecessary changes at the end of code freeze. |
Description:
Pin onnx version so that onnxruntime won't accidentally get broken because of a new ONNX release
Motivation and Context
ONNX defines some python interface(base classes)
We implemented these inferface.
However, nobody guarantee the inferface won't have backward incompatible changes in ONNX.