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

Nix build #277

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Nix build #277

wants to merge 1 commit into from

Conversation

sheepforce
Copy link

Currently the Haskell bindings are a little bit difficult to access with Nix. This is some work to build the tensorflow bindings with Nix and provide an overlay via a Nix flake that allows to use the Haskell packages of this repo via an overlay in other Nix projects.

  • Build all modules
    • tensorflow
    • tensorflow-core-ops
    • tensorflow-logging
    • tensorflow-opgen
    • tensorflow-ops
    • tensorflow-proto
    • tensorflow-records
    • tensorflow-records-conduit
    • tensorflow-test
  • shell.nix
  • default.nix

In the tensorflow-core-ops build I am getting:

tensorflow-core-ops> [1 of 1] Compiling TensorFlow.GenOps.Core ( dist/build/global-autogen/TensorFlow/GenOps/Core.hs, dist/build/TensorFlow/GenOps/Core.o, dist/build/TensorFlow/GenOps/Core.dyn_o )
tensorflow-core-ops> dist/build/global-autogen/TensorFlow/GenOps/Core.hs:69046:13: error:
tensorflow-core-ops>     Variable not in scope: num_side_inputs :: Int64
tensorflow-core-ops>       |
tensorflow-core-ops> 69046 |     pureOp [num_side_inputs] $ do

and I assume this is caused by having tensorflow-2.9.1 in nixpkgs but using some 2.3ish version as the submodule from which c_api.h is imported.
Using tensorflow-2.9.1 as a submodule makes things worse, unfortunately.

@google-cla
Copy link

google-cla bot commented Jul 27, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

cp -r $src/* .

chmod -R +rwx ./third_party
cp -r ${tensorflowSubModule} ./third_party/tensorflow
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I don't see a good option to get Nix accepting the submodules, it would be nice to use prev.libtensorflow.src instead tensorflowSubModule, so that the correct version also used in nixpkgs is provided. libtensorflow-2.9.1 from current nixpkgs gives compilation errors, however.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no opinion about this as I don't understand Nix enough and we have no CI around that. Fix it to your liking?

@mikesperber
Copy link
Contributor

@sheepforce While I support this work, note that the Tensorflow Haskell bindings have had regular Nix packages for a long time.

The current version has a blocking issue though:

#274

@sheepforce
Copy link
Author

While I support this work, note that the Tensorflow Haskell bindings have had regular Nix packages for a long time.

Yes, I am aware :) However, it is marked broken since a long time and does not actually build. I saw your PR attempting to fix it NixOS/nixpkgs#119411
The nixpkgs pulls the Hackage version from 2019 which misses all the fixes that have made it to this repo. I was hoping to get a more recent version directly from this repo. Having the Nix expressions here also gives the option to get a development environment with Nix.

The current version has a blocking issue though. #274

Could this also be caused by the Tensorflow version mismatch between nixpkgs and the tensorflow submodule here with c_api.h? I would love to help but I am not so fluent with C and how to call it via the Haskell FFI.

@mikesperber
Copy link
Contributor

mikesperber commented Jul 28, 2022

However, it is marked broken since a long time and does not actually build. I saw your PR attempting to fix it NixOS/nixpkgs#119411 The nixpkgs pulls the Hackage version from 2019

That's incorrect. It refers to github revision 568c9b6, which is pretty close to HEAD. Updating to HEAD is "just" a matter of resolving the above PR.

Could this also be caused by the Tensorflow version mismatch between nixpkgs and the tensorflow submodule here with c_api.h? I would love to help but I am not so fluent with C and how to call it via the Haskell FFI.

It's not out of the question, and that's what I looked for, too. However in testing I was pretty careful making sure the versions match.

@sheepforce
Copy link
Author

That's incorrect. It refers to github revision 568c9b6, which is pretty close to HEAD.

You are completely right, saw it in the PR. I've got confused by the version in nixpkgs being labelled 0.1.0.0

@blackgnezdo blackgnezdo marked this pull request as ready for review August 8, 2022 16:07
Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(approving to launch kokoro)

@Minnozz
Copy link
Contributor

Minnozz commented Jan 9, 2023

@sheepforce The num_side_inputs error seems to be a bug in the either the code generation or the protobuf definition of the op. You can add it to the blacklist to exclude it from generating bindings.

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

Successfully merging this pull request may close these issues.

7 participants