Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Remove obsolete rm since we upgraded to Pytorch 0.2. #101

Merged
merged 1 commit into from
Aug 8, 2017

Conversation

schmmd
Copy link
Member

@schmmd schmmd commented Aug 7, 2017

No description provided.

@schmmd schmmd requested a review from joelgrus August 7, 2017 23:03
@schmmd
Copy link
Member Author

schmmd commented Aug 7, 2017

@joelgrus are you able to give me instructions on how to reproduce the GOMP_4.0 not found error? Regardless, I'm pretty sure this change is needed.

@schmmd
Copy link
Member Author

schmmd commented Aug 7, 2017

I'm able to build a Docker image and pass all tests with this change.

@schmmd schmmd requested a review from DeNeutoy August 7, 2017 23:18
Copy link
Contributor

@DeNeutoy DeNeutoy left a comment

Choose a reason for hiding this comment

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

I don't have much context on why/if this is fixed by upgrading, but if they build, I guess they build. LGTM.

@joelgrus
Copy link
Contributor

joelgrus commented Aug 7, 2017

it's there because I ran into this problem: hughperkins/pytorch#28

but if it works in the new version, it works.

Copy link
Contributor

@joelgrus joelgrus left a comment

Choose a reason for hiding this comment

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

lgtm

@schmmd schmmd merged commit 241b275 into allenai:master Aug 8, 2017
@schmmd
Copy link
Member Author

schmmd commented Aug 8, 2017

@joelgrus @DeNeutoy that file actually no longer exists, and so the rm command was failing the Docker build. That's how I found it.

I saw the GitHub issue already. I just wasn't sure how you repro-ed.

@schmmd schmmd deleted the soft-rm branch August 8, 2017 00:11
@joelgrus
Copy link
Contributor

joelgrus commented Aug 8, 2017

I originally added that in because my docker builds were failing without it, which is what led me to the issue.

@schmmd
Copy link
Member Author

schmmd commented Aug 8, 2017

Huh, that seems surprising as it's near the end of the Dockerfile. In any case, we're up and running again now that this is merged. https://cloud.docker.com/swarm/allennlp/repository/docker/allennlp/allennlp-gpu/tags

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants