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

Convert LeNet-MNIST example to X10 #578

Merged
merged 2 commits into from
Jun 9, 2020

Conversation

Andr0id100
Copy link
Contributor

First of many PRs for #514.

Been busy with a few things lately hence the hiatus. Played around with X10 the last couple of days and I am really exited about the future of the S4TF project. Great work

@BradLarson
Copy link
Contributor

As a quick update: I checked this out on Linux and it works great there, however, we're running into some issues with X10 on macOS that we're looking into now. Specifically, this example segfaults under X10 on macOS with both our 0.9 release toolchain and the current nightlies. It's not a problem with what you have here, but something internal.

Before we merge this in, I want to see if we can fix this. If not, we may need to add selective compilation checks to use the TF eager device by default in this example on macOS so that it still works out of the box. I'll let you know what we find.

@RahulBhalley
Copy link
Contributor

Yup, I also experienced segmentation fault when executing on X10 on my macOS earlier.

Copy link
Contributor

@BradLarson BradLarson left a comment

Choose a reason for hiding this comment

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

Hope you don't mind, I pushed a modification to conditionally disable X10 support on macOS. We're not going to be able to fix the issue there with this model for a little bit, so it was best to get this in now.

Thanks for working on this, but before you move forward with other models, PR #586 will introduce a new and much cleaner training loop interface that will accommodate X10. I'll be working to migrate over our other models to that once that PR lands.

@BradLarson BradLarson merged commit 81390ad into tensorflow:master Jun 9, 2020
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.

4 participants