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

Update/Fix for classification models #1777

Closed
wants to merge 4 commits into from
Closed

Conversation

fayejf
Copy link
Collaborator

@fayejf fayejf commented Feb 20, 2021

  1. Change self.log(tensorboard_logs) and remove the 'log' part in the return requested in PR #1707
  2. Fix for logging for top-k acc metrics
  3. Fix tarred dataloader for speech regression support

Signed-off-by: fayejf <fayejf07@gmail.com>
Signed-off-by: fayejf <fayejf07@gmail.com>
Signed-off-by: fayejf <fayejf07@gmail.com>
@fayejf fayejf requested a review from titu1994 February 25, 2021 00:37
@okuchaiev
Copy link
Member

just to confirm that this is not a bugfix for rc1?

@fayejf
Copy link
Collaborator Author

fayejf commented Feb 25, 2021

@okuchaiev Part of this (Point 2) should be fixed in RC but this might raise conflict and not very essential (just logging), so we decide to merge it to main. I could split it and merge to RC if it's better.

@titu1994
Copy link
Collaborator

titu1994 commented Mar 4, 2021

Split this PR into two please. Bugfix to RC, logging to main. You can revert the RC patch from this PR and do it in another or vice-versa.

@fayejf
Copy link
Collaborator Author

fayejf commented Mar 10, 2021

@titu1994
Moved the following to RC in #1878. Could you please review it?

  1. Change self.log(tensorboard_logs) and remove the 'log' part in the return requested in PR before regresion
  2. Fix for logging for top-k acc metrics

Let wait for RC to merge to main and I will update self.log and bug fix for regression to avoid any conflict.

@fayejf
Copy link
Collaborator Author

fayejf commented Mar 12, 2021

Close this since first part has been merged in #1878 and second part #1907 is ready for review

@fayejf fayejf closed this Mar 12, 2021
@fayejf fayejf deleted the update_classification_log branch March 16, 2021 03:42
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.

3 participants