Fix comet logger to log after train #892
Conversation
|
Some tests failed, but I'm not really sure why. Looks like nothing related to what I change though... |
| @rank_zero_only | ||
| def finalize(self, status): | ||
| self.experiment.end() | ||
| self.reset_experiment() |
There was a problem hiding this comment.
What is the need for setting self._experiment to None?
There was a problem hiding this comment.
When calling self.experiment.end(), that experiment won't log any more data to Comet (that was issue #760 about). That's why, if you need to log any more data you need to create an ExistingCometExperiment. For example, to log data when testing your model after training, because when training is finalized CometLogger.finalize is called.
With this PR, this happens automatically in the CometLogger.experiment property, when self._experiment is set to None.
Tell me if anything is not clear yet :)
There was a problem hiding this comment.
I think it would be nice to write this your explanation also to code as a comment for future... 🤖
|
@fdelrio89 Thanks! The PR seems to work. |
| @rank_zero_only | ||
| def finalize(self, status): | ||
| self.experiment.end() | ||
| self.reset_experiment() |
There was a problem hiding this comment.
I think it would be nice to write this your explanation also to code as a comment for future... 🤖
Explains the need to use CometExistingExperiment in the CometLogger class after CometLogger.finalize.
|
Hello @fdelrio89! Thanks for updating this PR.
|
Good idea @Borda, I already uploaded this change to the PR. Let me know if there's anything else I can do to improve this code ✌️ |
* Fix comet logger to log after train * Add clarifying comment to COmetLogger code Explains the need to use CometExistingExperiment in the CometLogger class after CometLogger.finalize.
What does this PR do?
Fixes issue #760.
Updates the
CometLoggerclass for it to support logging data after the training is done. For this anExistingCometExperimentis created inside theCometLoggerobject once the training is finalized. When this happens,CometExperiment.endis called, which makes this object not to log any more data.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.