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

Fix Windows release builds. #590

Merged
merged 1 commit into from
Jan 26, 2021
Merged

Fix Windows release builds. #590

merged 1 commit into from
Jan 26, 2021

Conversation

AdamHillier
Copy link
Contributor

What do these changes do?

This PR (hopefully) fixes the Windows release builds, by copying in the most recent Windows build config from TensorFlow, using some extra commands that reduce memory usage, and increasing the size of the pagefile (swapfile).

How Has This Been Tested?

I will trigger a nightly release from this branch.

Benchmark Results

N/A.

Related issue number

Relies on #589.

@AdamHillier AdamHillier marked this pull request as draft January 25, 2021 11:23
@lgeiger lgeiger self-requested a review January 25, 2021 11:27
@AdamHillier
Copy link
Contributor Author

@AdamHillier
Copy link
Contributor Author

Test release run: https://github.com/larq/compute-engine/runs/1761598156

The Windows builds succeeded here, but the Linux ones failed. @lgeiger do you think this is related to this PR?

@lgeiger
Copy link
Member

lgeiger commented Jan 25, 2021

The Windows builds succeeded here, but the Linux ones failed. @lgeiger do you think this is related to this PR?

Great news on the Windows builds!

The Linux failures could either be related to #589 since we are now hardcoding the Python env variables or it could be due to a change in the Dockerfile.
It looks like tensorflow/tensorflow:custom-op-ubuntu16 was updated recently, it might be worth to pin it to tensorflow/tensorflow:2.2.0-custom-op-ubuntu16 instead and try again.
Our release setup is a bit hacky and needs to work around a few issues with the docker file, but I think moving to the tensorflow/tensorflow:2.4.0-custom-op-ubuntu16 might require more work, so I'd rather postpone this until we really need it (e.g. for Python 3.9 support).

Copy link
Member

@lgeiger lgeiger left a comment

Choose a reason for hiding this comment

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

Thank you so much for debugging this 🙏

.bazelrc Show resolved Hide resolved
.bazelrc Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
Base automatically changed from use-python-script-for-configure to master January 25, 2021 22:00
@AdamHillier
Copy link
Contributor Author

I ran a build with the updated code last night and everything is now green, including the Linux builds, so I think this is good to go: https://github.com/larq/compute-engine/actions/runs/510137328

@AdamHillier AdamHillier marked this pull request as ready for review January 26, 2021 10:04
@lgeiger lgeiger added the bug Something isn't working label Jan 26, 2021
Copy link
Member

@lgeiger lgeiger left a comment

Choose a reason for hiding this comment

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

Great! Thanks for fixing the build 🚀

@lgeiger lgeiger merged commit 95fd0b0 into master Jan 26, 2021
@lgeiger lgeiger deleted the fix-windows-builds branch January 26, 2021 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants