Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Migrate and switch to tensorflow-sys #6

Merged
merged 2 commits into from
Jul 7, 2016
Merged

Migrate and switch to tensorflow-sys #6

merged 2 commits into from
Jul 7, 2016

Conversation

IvanUkhov
Copy link
Contributor

@IvanUkhov IvanUkhov commented Jul 6, 2016

The pull request moves the tensorflow-sys crate to this repository and switches the main crate, tensorflow, from libtensorflow-sys to tensorflow-sys. There are a couple of things to note before the pull request can be merged.

Most of the crates in the Rust ecosystem are dual licensed under Apache and MIT. I suppose this project wants to stick to Apache only, and, therefore, I adjusted Cargo.toml accordingly.

I use the term “bindings” instead of “low-level bindings” in the description of tensorflow-sys since, in my opinion, “bindings” already implies bear minimum. Idiomatic Rust codes should be called “wrappers” or “interfaces” in this context, but I guess it’s a matter of taste.

I don’t write any work-in-progress warnings because it’s redundant due to the semantic versioning, which this community loves so much. Before the 1.0 release anything can happen. I personally think that the tensorflow_unstable of tensorflow should be removed.

The documentation’s address is set to https://google.github.io/tensorflow-rust, where it’ll hopefully appear once Travis CI has been properly configured and starts to populate the gh-pages branch.

Please let me know if any additional changes are needed. Thanks!

Regards,
Ivan

.arg(format!("--jobs={}", get!("NUM_JOBS")))
.arg("--compilation_mode=opt")
.arg(format!("{}:{}", LIBRARY, TARGET)));
let source = source.join("bazel-out/local-opt/bin/tensorflow");
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use "bazel-bin/tensorflow", then it doesn't include the build configuration ("local-opt").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Fixed.

@adamcrume
Copy link
Contributor

I chose the Apache license because that's what the main TensorFlow project is using. I don't have a problem with the MIT license, and it does seem popular on crates.io (http://paul.woolcock.us/posts/crates-io-license-survey.html), but I'd have to check with the Google open source licensing folks before changing it. Is there any specific advantage to dual-licensing? The Apache and MIT licenses are both permissive and pretty similar, as I understand it.

I agree in theory about the version number already signifying instability, but I'm afraid that many users might not pay attention to that. The tensorflow_unstable feature forces them to pay attention, especially since this is pre-alpha ("not even written yet"). I also plan on scaling it back in scope as the API stabilizes. I'd really like to use #[unstable], except for the fact that it doesn't exist. I think the main blocker in removing tensorflow_unstable is that we can't generate graphs, yet, and adding that is a big enough change to risk fundamental changes. I'm fine with tensorflow-sys not being gated behind tensorflow_unstable, since the low-level bindings should be pretty stable and complete already.

The changes look good, though. I'll go ahead and merge them, but I won't have time to set up Travis tonight.

@adamcrume adamcrume merged commit 5495f12 into tensorflow:master Jul 7, 2016
@IvanUkhov
Copy link
Contributor Author

IvanUkhov commented Jul 7, 2016

@adamcrume, the motivation behind the Apache–MIT combo can be found in relicense-assisntat. Around half a year ago, this script went through the crates on crates.io and opened issues about relicensing in the corresponding repositories. Many people agreed and relicensed their crates.

ramon-garcia pushed a commit to ramon-garcia/tensorflow-rust that referenced this pull request May 20, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants