[go: nahoru, domu]

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

Would it be possible to relax the requirements? #648

Closed
BastianZim opened this issue Dec 26, 2021 · 6 comments
Closed

Would it be possible to relax the requirements? #648

BastianZim opened this issue Dec 26, 2021 · 6 comments

Comments

@BastianZim
Copy link

Hi,

this issue is primarily along the lines of tensorflow/tensorflow@a90383a.
I was wondering if the tight pins in requirements.txt are primarily due to functionality or solvability? Reason is, that I'm thinking about adding this package to conda-forge but the restrictions currently make this difficult.
It seems like things like the numpy pin are primarily to make solving easier but I just wanted to double check if this is correct?
Thanks!

@MichaelBroughton
Copy link
Collaborator

Hi @BastianZim thanks for opening the issue. There are a few reasons why we pin specific versions (most being for functionality)

  1. We pin TensorFlow==2.5.1 because all of the C++ code we have inside of TFQuantum (and all other custom C++ tensorflow repos for that matter) rely on ABI stability. If you were to install the latest TFQ and install any version of TensorFlow other than 2.5.1 you might see symbol table lookup issues like this one: NotFoundError: _tfq_simulate_ops.so #180 .
  2. The proto version requirements we inherit from TF. I think these could be relaxed. Maybe try opening a PR to experiment ?
  3. The numpy pin is one we also inherit from TF.

Also note we use requirements.txt for setting up a development environment, but the requirements we impose on the user can be found in setup.py.

Most of these requirements trickle down to us from TF and the known toolchain + flags that are used to compile it. On conda I believe that the process isn't identical and it could wind up that you might need to compile TFQ using whatever flags match the ones on conda. Looking here: https://anaconda.org/conda-forge/tensorflow/files?version it looks like conda doesn't host TF 2.5.1, so that may already be a non-starter, until we can get an upgrade on the version of TF we depend on here in TFQ.

@BastianZim
Copy link
Author
BastianZim commented Jan 3, 2022

Hi @MichaelBroughton thanks for the reply. Let me answer in a list as well, just to make it easier.

  1. Ok makes sense. I wasn't sure if Tensorflow follows SemVer where the patches shouldn't matter but it you require an exact pin then of course it's different. And yes, conda-forge only offers later versions (Starting at 2.6.0 with 2.5 skipped). Since Tensorflow is one of the most difficult packages to distribute, downgrading is probably not going to happen so I'll have to wait until that is updated.
  2. Will do – thanks for the info. Do the tests cover the proto part already or is there a best practise way to test that?
  3. Ok great then I will relax that as conda will then solve the environment using the Tensorflow specs.

requirements.txt

Ahh ok I didn't find it originally but just saw that it's under /release/setup.py. That makes it much easier.

On conda I believe that the process isn't identical and it could wind up that you might need to compile TFQ using whatever flags match the ones on conda.

Yes, conda-forge has it's own compiler infrastructure which is used for all packages and a bazel toolchain which is used to generate the respective flags. This is done to prevent the exact situation you described where different packages might end up not working together.

@BastianZim
Copy link
Author

I would also have one question regarding the compilation: Proto etc. is generally, in my experience, required during compilation and then when running the package, but from what I can see the only thing required here (Besides the compiler toolchain) is Tensorflow. Is that correct?

@MichaelBroughton
Copy link
Collaborator

We use the version of protoc provided by TensorFlow in our project (via bazel dependencies) when developing code:

  1. We use this protobuf for Codegen + Compilation in C++
  2. We also use it for Codegen in python
    When actually running code all we require is that you use a version of this that happens to also work with the current version of protoc used in TensorFlow. Right now protoc 3.9.2 is used, but for the ease of our users we can allow the pip install to slide all the way up to 3.17.3

This is a problem TensorFlow also has and is working on fixing here

@lockwo
Copy link
Contributor
lockwo commented Aug 24, 2022

Any updates on this issue or can it be closed?

@mihaimaruseac
Copy link

TF fixed the protobuf issue in tensorflow/tensorflow@84f4092

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants