-
Notifications
You must be signed in to change notification settings - Fork 422
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
Add InfoLM
#915
Add InfoLM
#915
Conversation
* Add module metric InfoLM * Apply partially some suggestions from code review * Fix some missing imports
Codecov Report
@@ Coverage Diff @@
## master #915 +/- ##
======================================
- Coverage 94% 94% -0%
======================================
Files 182 185 +3
Lines 8147 8389 +242
======================================
+ Hits 7654 7880 +226
- Misses 493 509 +16 |
@justusschock @SkafteNicki Thanks a lot for your reviews, I'll resolve the remaining issues tomorrow. @Borda Not sure how to fix the issue with those daemonic processes having children -- gonna probe this in more depth tomorrow as well :] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @stancld,
I did a bit of digging and it seem the problem is using multiple number of workers in the dataloader, as this will try to spawn multiple processes within each ddp process (they are spawned for testing) which is not allowed. It seems to be working for BERTScore so I am not sure why this is not working for this metric
* Skip tests if transformers are not available * Mark tests as skip if there's again any connection issue to HF Hub
@Borda Mind have a look at some of your previous comments if they're now settled? :] |
I think the problem arises when |
@stancld good with me 🎉 how about the very last comments? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@stancld mind checking the remaining comments and mar as resolved if it so... then it ll be merged 🎉 |
What does this PR do?
Fixes #849
Before submitting
I've prepared a personal repository with scripts/instructions for reproducing test results -> will be also useful for #852 and #854 if the original package installation will not be available.
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 🙃