Skip to content
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

use tensorflow_probability[tf] extra to match versions #827

Closed
wants to merge 3 commits into from

Conversation

egpbos
Copy link
Member

@egpbos egpbos commented Jul 29, 2024

Fixes #826.

Related to #434 and possibly #727.

@egpbos egpbos requested a review from cwmeijer July 29, 2024 13:57
@egpbos
Copy link
Member Author

egpbos commented Jul 29, 2024

The torchtext failures seem unrelated, since they also affect the other PR I just made which changes nothing in the build system (#828). Annoying, though, guess we'll have to wait for a resolution of this.

@egpbos
Copy link
Member Author

egpbos commented Jul 29, 2024

Oef, waiting for resolution might not work out, torchtext development has been discontinued... pytorch/text#2250

What of torchtext are we using?

@egpbos
Copy link
Member Author

egpbos commented Jul 29, 2024

Copy-pasting the actual error message for other Googlers encountering this problem:

tests/test_kwargs.py:6: in <module>
    from tests.utils import get_dummy_model_function
tests/utils.py:5: in <module>
    from torchtext.vocab import Vectors
/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/torchtext/__init__.py:18: in <module>
    from torchtext import _extension  # noqa: F401
/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/torchtext/_extension.py:64: in <module>
    _init_extension()
/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/torchtext/_extension.py:58: in _init_extension
    _load_lib("libtorchtext")
/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/torchtext/_extension.py:50: in _load_lib
    torch.ops.load_library(path)
/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/torch/_ops.py:1295: in load_library
    ctypes.CDLL(path)
/opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/ctypes/__init__.py:374: in __init__
    self._handle = _dlopen(self._name, mode)
E   OSError: /opt/hostedtoolcache/Python/3.9.19/x64/lib/python3.9/site-packages/torchtext/lib/libtorchtext.so: undefined symbol: _ZN5torch3jit17parseSchemaOrNameERKSs

@SarahAlidoost
Copy link
Member

Oef, waiting for resolution might not work out, torchtext development has been discontinued... pytorch/text#2250

What of torchtext are we using?

see #837

@cwmeijer
Copy link
Member

cwmeijer commented Aug 7, 2024

What of torchtext are we using?

image

Only Vectors to create a vocabulary with string to integer conversions, and get_tokenizer to instantiate a spacy tokenizer. Maybe we can find that functionality elsewhere. SpaCy may provide both functionalities. It is already a dependency in DIANNA.

@cwmeijer
Copy link
Member

Closing and reopening in an attempt to trigger rerunning the checks.

@cwmeijer cwmeijer closed this Sep 11, 2024
@cwmeijer cwmeijer reopened this Sep 11, 2024
@cwmeijer cwmeijer closed this Sep 18, 2024
@cwmeijer cwmeijer reopened this Sep 18, 2024
@elboyran
Copy link
Contributor

Still many checks not passing. @cwmeijer what to do with this PR?

@elboyran elboyran closed this Sep 18, 2024
@elboyran elboyran reopened this Sep 18, 2024
@egpbos egpbos force-pushed the 826_tensorflow_dependency branch from 24f3d05 to 8af438c Compare September 19, 2024 09:31
@egpbos
Copy link
Member Author

egpbos commented Sep 19, 2024

Rebased on main.

@egpbos
Copy link
Member Author

egpbos commented Sep 19, 2024

Now we have other failures, which seem to be caused by keras 3, which has dropped access to the (previously private) keras.src module and some other stuff. However, I don't see this being used directly in DIANNA code itself, so the issues are probably in some dependencies that did not get updated and that did not specify upper limits to their keras dependency...

Keras 3 introduced a number of backwards incompatible changes that make some dependencies (shap, rise) fail.
@egpbos egpbos force-pushed the 826_tensorflow_dependency branch from b180dac to 03b0254 Compare September 19, 2024 20:40
@egpbos
Copy link
Member Author

egpbos commented Sep 19, 2024

Wow what a can of worms. This is causing many different errors, like a pip solver error on Windows:

The conflict is caused by:
    dianna 1.6.0 depends on keras<3
    dianna[dev] 1.6.0 depends on keras<3
    tensorflow-intel 2.16.1 depends on keras>=3.0.0

On Ubuntu there is no conflict, but the tests error out, so I'm guessing there's still an incompatible set of packages getting installed.

If nobody else has ideas feel free to close this PR, it is not very urgent for me, just a nice to have.

@elboyran
Copy link
Contributor

Wow what a can of worms. This is causing many different errors, like a pip solver error on Windows:

The conflict is caused by:
    dianna 1.6.0 depends on keras<3
    dianna[dev] 1.6.0 depends on keras<3
    tensorflow-intel 2.16.1 depends on keras>=3.0.0

On Ubuntu there is no conflict, but the tests error out, so I'm guessing there's still an incompatible set of packages getting installed.

If nobody else has ideas feel free to close this PR, it is not very urgent for me, just nice to have.

@cwmeijer what do you think - shall we close it? It's pity not to use the elegant solution, but if it causes many problems...

@cwmeijer cwmeijer added the standup Temp label- for disscussion with the team next standup label Oct 2, 2024
@cwmeijer
Copy link
Member

cwmeijer commented Oct 2, 2024

I agree this would be nice to solve, but I expect this to take quite some time to solve. I propose to not solve this right now.

@cwmeijer cwmeijer closed this Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standup Temp label- for disscussion with the team next standup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tensorflow lower limit conflicts
4 participants