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

Make N_CPUS check in ci_sanity.sh work on MacOS #29711

Merged
merged 1 commit into from Jun 18, 2019

Conversation

frreiss
Copy link
Contributor

@frreiss frreiss commented Jun 12, 2019

The pylint sections of tensorflow/tools/ci_build/ci_sanity.sh do not run on MacOS because they contain code that assumes the existence of /proc/cpuinfo.

This is a problem for Mac users because pylint is extremely slow on Docker on Mac -- over 12 hours to to run a single pass of linting on my machine. The same pylint check runs in 15 minutes outside of Docker.

The changes in this PR allow the linting portions of ci_sanity.sh to run on MacOS with Docker. After my modifications, ci_sanity.sh uses the POSIX standard command getconf to query the number of cores if /proc/cpuinfo does not exist.

To get reliable results when running pylint locally, users will need a Python environment that is close to that of the TensorFlow Docker images. I have written a script that will set up such an environment. My script is currently checked into my TensorFlow config files repository at https://github.com/frreiss/tf-dev-conf/blob/master/lintenv.sh. I would be happy to contribute this script if there's interest.

Make CPU check work on MacOS

Cleaned up logic of CPU test
@tensorflow-bot tensorflow-bot bot added the size:XS CL Change Size: Extra Small label Jun 12, 2019
@gbaned gbaned self-assigned this Jun 13, 2019
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Jun 13, 2019
@gbaned gbaned requested a review from av8ramit June 13, 2019 05:03
PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Jun 15, 2019
@tensorflow-bot tensorflow-bot bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Jun 15, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Jun 15, 2019
@tensorflow-copybara tensorflow-copybara merged commit 269e008 into tensorflow:master Jun 18, 2019
PR Queue automation moved this from Approved by Reviewer to Merged Jun 18, 2019
tensorflow-copybara pushed a commit that referenced this pull request Jun 18, 2019
@frreiss frreiss deleted the issue-n-cpus branch December 2, 2019 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process size:XS CL Change Size: Extra Small
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants