-
Notifications
You must be signed in to change notification settings - Fork 220
Upgrade for TensorFlow 2.7.0 #395
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
Conversation
|
Thanks a lot @saudet , I'll rebase it on the new master branch and I'll take care of classifying the new ops, looks like the |
|
Patches from custom gradients are going to need to be looked at too, I think one or two of my core PRs made it into this release. |
|
Yes no problem @rnett , I've noticed that and already fixed it |
|
When building on my Mac, I have this garbled code being added at the end of a JavaCPP generated class I guess if you did not encountered this issue before @saudet , then maybe this mapping was added by @rnett in his last custom gradient PR that has been just merged? But anyway, what would be the right way to fix this? Right now, there's only this in the TF presets: |
|
Ah found it, by adding it to the skip list... |
|
Yeah, my guess is that was added in tensorflow core this release and as such wasn't on our ignore list. |
0ba57ac to
5b58c58
Compare
|
@saudet , I've pushed an updated version into your branch which is rebased with the latest changes + I've reclassified the ops a little bit (looks like the I'm having trouble to build locally but for various strange reasons, including timeouts, so I'm giving it a try on the CI build instead. |
|
Ok, I'm not too sure what to do here.. I'm having also strange errors in the CI Build but they seem related to the Bazel cache timing out... https://github.com/tensorflow/java/runs/4194390356?check_suite_focus=true I'll give it one more run but if someone can checkout locally that branch and confirm again that it builds successfully for them I would be more confident to merge it. |
|
I'll run it tonight and see. |
|
Oh, just found one source of confusion. One of your patch @rnett is no longer required, they applied the same change in 2.7, I've initially thought you had reversed it... Let's try again without it now. |
Ok now I understand what you meant by this :D Sorry! |
|
Here's what I got, likely because of the patch: Trying again. |
|
The bazel build is working for me now, but I get javacpp compliation errors. Likely more missing skips for new stuff. Errors: https://gist.github.com/rnett/c7048d0efc355e2a535946de92f2d073 |
|
@saudet can you please take a look at this? https://github.com/tensorflow/java/runs/4199332671?check_suite_focus=true#step:4:1870 Windows build also failed and Linux timed out... I'll relaunch it tomorrow morning to give you some time to review these errors, thanks |
Ok, it's probably stuff that came with your custom gradient PR since Samuel was able to do a full build before rebasing. Can you please take a look at it? |
|
Yes, yes, working on it now.. I'm having a weird issue with Maven not recompiling properly, which is annoying, debugging that first before pushing. |
|
Some probably are, but I don't think it's just that, look at this one: That's not even from our code, its from tensorflow core. |
|
It looks like we can't rely on the order of the plugins when overriding their configurations like with maven-compiler-plugin: |
|
I can't commit to the branch, but |
|
I'm also not seeing the |
Also fix execution order plugins for javacpp-parser
|
Ok, everything fixed! I think. It's working for me anyway. :) |
|
Good job @saudet , looks like Mac and Linux are happy now. Windows on the other hand... This looks like a strange path... I don't even know where to start looking. WSL again? Looks like the |
karllessard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, CI build looks promising, let's give it a shot, thanks @saudet !
|
@saudet , all native builds pass but the Linux-GPU one, other than a bunch of cache timeouts, we're getting this error: https://github.com/tensorflow/java/runs/4252113624?check_suite_focus=true#step:5:4429 Any idea how to solve this? |
|
Looks like a compiler bug, probably fixed in recent versions. Now that we
can use a not super old version of CUDA, we should be able to upgrade to
devtoolset-9 or 10.
|
|
Trying with 9 right now… |
|
Great, upgrading to devtoolset-9 did the trick, we’re done here! |
Builds fine and all tests pass for me on Linux.
I've also updated the proto patch and ran the java_api_import tool, which generated a couple of things without prompting...