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 weird docker error #2581

Merged
merged 3 commits into from
Jun 4, 2024
Merged

Fix weird docker error #2581

merged 3 commits into from
Jun 4, 2024

Conversation

SkafteNicki
Copy link
Member

@SkafteNicki SkafteNicki commented Jun 3, 2024

What does this PR do?

Fixes #2559

I have no idea what the actual issue is here. It is therefore up for discussion if this PR should be merged.
Apparently, using torchmetrics inside a docker image can lead to the following error:

RuntimeError:
DataLoader worker (pid(s) 104) exited unexpectedly 

which can happen due to so many reasons in my experience. See the issue for more details, but I was able to reproduce the issue on v1.4.0 of torchmetrics but not on v1.3.2, thus after running git bisect I was able to narrow it down to this commit cd7ccfc. Then removing each line one by one I could narrow it down to the data loader having some weird interaction with the requirement cache.

I do not really think this is anything we can test for.

Before submitting
  • Was this discussed/agreed via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?
PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃


📚 Documentation preview 📚: https://torchmetrics--2581.org.readthedocs.build/en/2581/

@SkafteNicki SkafteNicki added the bug / fix Something isn't working label Jun 3, 2024
@SkafteNicki SkafteNicki added this to the v1.3.x milestone Jun 3, 2024
@SkafteNicki SkafteNicki marked this pull request as ready for review June 4, 2024 07:38
Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

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

Looks strange but if it helps...

Copy link

codecov bot commented Jun 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69%. Comparing base (f1a2be7) to head (6e1e877).
Report is 95 commits behind head on master.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #2581   +/-   ##
======================================
  Coverage      69%     69%           
======================================
  Files         314     314           
  Lines       17695   17696    +1     
======================================
+ Hits        12197   12198    +1     
  Misses       5498    5498           

@mergify mergify bot added the ready label Jun 4, 2024
@Borda Borda merged commit b0da59e into master Jun 4, 2024
59 of 61 checks passed
@Borda Borda deleted the bugfix/weird_docker_error branch June 4, 2024 13:53
Borda pushed a commit that referenced this pull request Aug 2, 2024
(cherry picked from commit b0da59e)
Borda pushed a commit that referenced this pull request Aug 2, 2024
(cherry picked from commit b0da59e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug / fix Something isn't working ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataLoader worker is killed in Docker
2 participants