python3Packages.tensorflow: Fix numpy==1.20 support #130022
Conversation
bergkvist
force-pushed
the
bergkvist/tensorflow-numpy-1.20
branch
from
b3db322
to
a32b89b
Compare
bergkvist
force-pushed
the
bergkvist/tensorflow-numpy-1.20
branch
from
a32b89b
to
c71cd8e
Compare
bergkvist
changed the title
| +from tensorflow.python.ops import math_ops | ||
| # go/tf-wildcard-import | ||
| # pylint: disable=wildcard-import | ||
| from tensorflow.python.ops.gen_array_ops import * | ||
| @@ -2897,7 +2898,7 @@ def matrix_set_diag( | ||
|
|
||
| def _constant_if_small(value, shape, dtype, name): | ||
| try: | ||
| - if np.prod(shape) < 1000: | ||
| + if math_ops.reduce_prod(shape) < 1000: | ||
| return constant(value, shape=shape, dtype=dtype, name=name) |
There was a problem hiding this comment.
I feel like this should probably be upstreamed, there's nothing nixpkgs specific in it.
There was a problem hiding this comment.
I agree that this fix feels like it belongs in the Tensorflow repository. (or perhaps even as a change/fix to np.prod in the Numpy repository)
There have been open PRs to resolve the issue upstream/in the Tensorflow repo for more than 2 months. Supporting Numpy 1.20 doesn't seem to be a high priority for the Tensorflow team - since Numpy 1.19 is working fine with Tensorflow.
Numpy 1.20-support is pretty much essential in order for Tensorflow to work within the nixpkgs ecosystem, since nixpkgs currently ships NumPy 1.20 (pkgs.python3.pkgs.numpy). Downgrading NumPy to 1.19 in nixpkgs also seems like a risky option - since it might break something else depending on 1.20 or higher.
Shipping multiple versions of the same Python library within nixpkgs is also not a good option - since any environment that depends on multiple versions of the same python-library will break. That said, there is some interesting discussion around how to make it work here: https://discourse.nixos.org/t/allowing-multiple-versions-of-python-package-in-pythonpath-nixpkgs/3849
bergkvist
force-pushed
the
bergkvist/tensorflow-numpy-1.20
branch
from
c71cd8e
to
fe76d16
Compare
|
Upstream seem to have fixed this via tensorflow/tensorflow#47957 |
|
@risicle Okay - nice! Does that means we should close this pull request? (Once we verify that Tensorflow in nixpkgs is working) I guess an alternative would be to modify this PR to bump the Tensorflow version instead of patching it with the fix. |
|
Either works for me. |
|
Fixed in #132099 |
Motivation for this change
Fixes #128829
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)