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.

Sign up for GitHub

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

Jump to bottom

Fix trainer.predict(return_predictions=False) does not track batch_indices #13629

Merged
merged 5 commits into from
Jul 18, 2022
Merged

Fix trainer.predict(return_predictions=False) does not track batch_indices #13629

merged 5 commits into from
Jul 18, 2022

Conversation

georgestein
Copy link
Contributor

@georgestein georgestein commented Jul 12, 2022 •
edited by carmocca
Loading

What does this PR do?

Fixes #13580

@Borda Borda changed the title Pull request for fixing issue #13580 Fix Trainer.predict(return_predictions=False) does not track batch_indices Jul 12, 2022
rohitgr7
rohitgr7 reviewed Jul 13, 2022
View reviewed changes
Copy link
Contributor

@rohitgr7 rohitgr7 left a comment

Choose a reason for hiding this comment

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

looks good. Can you add a changelog entry and a small unit test to validate the change?

@rohitgr7 rohitgr7 added feature Is an improvement or enhancement callback: prediction writer community This PR is from the community labels Jul 13, 2022
@rohitgr7 rohitgr7 added this to the pl:1.7 milestone Jul 13, 2022
@rohitgr7 rohitgr7 changed the title Fix Trainer.predict(return_predictions=False) does not track batch_indices Fix trainer.predict(return_predictions=False) does not track batch_indices Jul 14, 2022
@awaelchli
Copy link
Contributor

Why is it labelled as feature?

@carmocca carmocca added bug Something isn't working and removed feature Is an improvement or enhancement labels Jul 15, 2022
@carmocca carmocca modified the milestones: pl:1.7, pl:1.6.x Jul 15, 2022
@mergify mergify bot added the ready PRs ready to be merged label Jul 15, 2022
@Borda Borda enabled auto-merge (squash) July 17, 2022 18:20
@Borda Borda requested a review from rohitgr7 July 17, 2022 18:21
@Borda Borda merged commit 0449e86 into Lightning-AI:master Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rohitgr7 rohitgr7 rohitgr7 approved these changes

@carmocca carmocca carmocca approved these changes

@awaelchli awaelchli awaelchli approved these changes

@tchaton tchaton Awaiting requested review from tchaton tchaton is a code owner

@justusschock justusschock Awaiting requested review from justusschock justusschock is a code owner

@williamFalcon williamFalcon Awaiting requested review from williamFalcon

@Borda Borda Awaiting requested review from Borda Borda is a code owner

@SeanNaren SeanNaren Awaiting requested review from SeanNaren

@kaushikb11 kaushikb11 Awaiting requested review from kaushikb11

Assignees

@rohitgr7 rohitgr7

Labels
bug Something isn't working callback: prediction writer community This PR is from the community ready PRs ready to be merged trainer: predict
Projects
None yet
Milestone
pl:1.6.x
5 participants
@georgestein @awaelchli @carmocca @rohitgr7 @Borda