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

Refactor RunningStage routing mechanism #5593

Closed
tchaton opened this issue Jan 20, 2021 · 3 comments · Fixed by #4945
Closed

Refactor RunningStage routing mechanism #5593

tchaton opened this issue Jan 20, 2021 · 3 comments · Fixed by #4945
Labels
help wanted Open to be worked on refactor
Milestone
pl:1.3

Comments

@tchaton
Copy link
Contributor

tchaton commented Jan 20, 2021 •
edited by carmocca
Loading

🚀 Feature

Motivation

Currently, tes / predict function is relying on an ENV variable to set testing to True.

The logic should be simplified and cleaned up.

Pitch

Additional context

No we can't in the current state of the codebase. Trainer.predict relies on self.trainertesting to be True for routing the code in the test part of the evaluation loop, and if we set self._running_stage = RuningStage.PREDICTING, it would route to validation and we won't gather the prediction.
I plan to properly refactor routing in a follow up PR as it is currently a big mess.

Originally posted by @tchaton in #5579 (comment)
@tchaton tchaton added feature Is an improvement or enhancement help wanted Open to be worked on labels Jan 20, 2021
@tchaton tchaton added this to the 1.2 milestone Jan 20, 2021
@tchaton tchaton changed the title Clean fit, test routing mechanism Clean fit, test, predict routing mechanism Jan 20, 2021
@rohitgr7
Copy link
Contributor

which env variable?

@carmocca carmocca mentioned this issue Jan 26, 2021
Closed
@carmocca carmocca changed the title Clean fit, test, predict routing mechanism Refactor RunningStage routing mechanism Jan 26, 2021
@carmocca carmocca removed the feature Is an improvement or enhancement label Jan 26, 2021
@edenlightning edenlightning modified the milestones: 1.2, 1.3 Feb 8, 2021
@awaelchli
Copy link
Contributor

@carmocca I believe this can be closed because you have worked on this just recently in #4945, right?

@carmocca
Copy link
Contributor

carmocca commented Mar 8, 2021

Yes!

@carmocca carmocca closed this as completed Mar 8, 2021
@carmocca carmocca mentioned this issue Mar 8, 2021
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Open to be worked on refactor
Projects
None yet
Milestone
pl:1.3
5 participants
@awaelchli @carmocca @tchaton @rohitgr7 @edenlightning