-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Fix bugs in RBM notebooks #1581
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov Report
@@ Coverage Diff @@
## staging #1581 +/- ##
===========================================
- Coverage 61.89% 61.88% -0.02%
===========================================
Files 84 84
Lines 8458 8458
===========================================
- Hits 5235 5234 -1
- Misses 3223 3224 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@@ -60,7 +60,7 @@ | |||
"System version: 3.7.11 (default, Jul 27 2021, 14:32:16) \n", | |||
"[GCC 7.5.0]\n", | |||
"Pandas version: 1.3.4\n", | |||
"Tensorflow version: 2.6.1\n" | |||
"Tensorflow version: 1.15.5\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @pradjoshi can you merge the latest stating to your branch, reinstall using TF 2 and rereun again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @miguelgfierro I have reinstalled recommenders using TF2 and rerun the notebooks.
View / edit / reply to this conversation on ReviewNB miguelgfierro commented on 2021-12-15T08:54:38Z Same comment as before with the verbosity pradnyeshjoshi commented on 2021-12-15T16:54:45Z Andreas resolved this. |
View / edit / reply to this conversation on ReviewNB miguelgfierro commented on 2021-12-15T08:54:38Z we need to revisit the change in metrics, maybe in a different PR. Is there an issue to track this? pradnyeshjoshi commented on 2021-12-15T16:54:14Z I have created a task for it. Should I create an issue on GitHub instead? |
View / edit / reply to this conversation on ReviewNB miguelgfierro commented on 2021-12-15T08:54:39Z Line #41. print("Pandas version: {}".format(pd.__version__)) maybe here it would be good to also add tf version pradnyeshjoshi commented on 2021-12-15T16:58:41Z Done. |
View / edit / reply to this conversation on ReviewNB miguelgfierro commented on 2021-12-15T08:54:40Z This is super verbose. Ideally what we want is just the old output, with the metrics loop: training epoch 0 rmse 0.961369 training epoch 10 rmse 0.902065 training epoch 20 rmse 0.864718 training epoch 30 rmse 0.848415 done training, Training time 10.9566452 Train set accuracy 0.3561482 Test set accuracy 0.3516242 It looks the command to show only the errors is not working: tf.get_logger().setLevel('ERROR') # only show error messages
Maybe there is a new command in TF. If you can't find it, it is better to manually erase the cell
pradnyeshjoshi commented on 2021-12-15T16:52:10Z Looks like Andreas resolved this already! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pradnyeshjoshi to me this looks ok to go, after the minor comments on logs.
@anargyri do you want to include this in the release?
I don't know, maybe if there is time. First we need to resolve the tests that fail in the release. |
Yes, this is strange. Other algos like deeprec don't have this amount of output. Maybe you could check what the difference is from those. |
I think I fixed it according to tensorflow/tensorflow#18146 |
…s into pradjoshi/test_rbm
Looks like Andreas resolved this already! View entire conversation on ReviewNB |
I have created a task for it. Should I create an issue on GitHub instead? View entire conversation on ReviewNB |
Andreas resolved this. View entire conversation on ReviewNB |
Done. View entire conversation on ReviewNB |
The task should be enough. |
Description
Related Issues
Checklist:
staging branch
and not tomain branch
.