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

Make accuracy and loss values available for upstream use-cases #662

Closed
wants to merge 3 commits into from

Conversation

vkuznet
Copy link

@vkuznet vkuznet commented Sep 1, 2020

Quite often we need accuracy and loss values in upstream code, e.g. to plot them. This PR address this issue.

@BradLarson
Copy link
Contributor

Thanks for the suggestion, but I don't think that adding logging functionality within the visual progress bar is the right place for this. That progress bar should only be a visual element, and is only there if you need to have a display of how training is going.

Logging (to CSV, text, TensorBoard, etc.) should happen within other callbacks built for that purpose. That would make it easy for the end user to know what functionality to add. If they just wanted logging, we don't want to have them need to use a visual display if they don't want it.

The idea is that these mechanisms would focus around a central TrainingStatistics callback that would extract the statistics once (to avoid the overhead of repeatedly materializing loss and other tensors) and then provide them to all consumers of stats. The initial architecture I used for the progress bar (it hosting an instance of TrainingStatistics and calling it directly) isn't going to achieve this, so we need to find a way to share a common TrainingStatistics callback from multiple logging or display callbacks while not exposing that implementation detail to the end user.

Again, thanks for suggesting this approach, but I'd like to find a more extensible solution for the problem.

@vkuznet
Copy link
Author

vkuznet commented Sep 2, 2020

Thanks for feedback. I agree, I just found that this use-case is not yet covered and I used a a place which had this info. But I'm totally fine to put it in more generic block. Feel free to close/reject PR , but it would be nice to have an open issue on this subject of having access to this info from upstream code.

@BradLarson
Copy link
Contributor

I just added issue #663 to track this, hopefully that includes all the relevant information and needs. I'll close this out in favor of whatever we can do to address the needs of that issue.

Again, thanks for bringing this up and reminding us about the need.

@BradLarson BradLarson closed this Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants