Remove Theano from Week 5 #348

yhn112 · 2020-04-10T20:06:36Z

Fixes #174. Part of #258.

review-notebook-app · 2020-04-10T20:06:42Z

Check out this pull request on

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

dniku

Cleanup notebook metadata
Prettify Python code
Remove redundant outputs
Fix Colab initialization

dniku

This is a superficial review. I haven't launched the code or even opened the notebook, I've only looked at the diff. The whole thing looks close to finished. I'll test whether it actually works later.

dniku · 2020-04-14T12:45:32Z

week05_explore/bayes.py

-"""
-A single-file module that makes your lasagne network into a bayesian neural net.
-Originally created by github.com/ferrine , rewritten by github.com/justheuristic for simplicity
+import torch


Since this file is converted from someone's module, I believe credit should be given to the original author.

dniku · 2020-04-14T12:46:01Z

week05_explore/bayes.py

+            self.bias.data.zero_()
+
+    def forward(self, x):
+


redundant new line

dniku · 2020-04-14T12:46:42Z

week05_explore/bayes.py

+        # Local reparameterization trick
+        out = mean + std * epsilon
+
+


redundant new line

dniku · 2020-04-14T12:48:14Z

week05_explore/week5.ipynb

@@ -2,7 +2,7 @@
 "cells": [
  {
   "cell_type": "code",
-   "execution_count": null,
+   "execution_count": 1,


Please clean up these. Use my script or Jupyter's native feature (cells -> execution counts -> clean all I believe)

dniku · 2020-04-14T12:48:58Z

week05_explore/week5.ipynb

+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "def calc_kl(model):\n",


This is already implemented in ModuleWrapper.forward, no?

dniku · 2020-04-14T12:49:14Z

week05_explore/week5.ipynb

+    "    for module in model.modules():\n",
+    "        if hasattr(module, 'kl_loss'):\n",
+    "            kl = kl + module.kl_loss()\n",
+    "    return kl/10000"


dniku · 2020-04-14T12:49:37Z

week05_explore/week5.ipynb

-    "                                          updates=updates,\n",
-    "                                          allow_input_downcast=True)\n",
+    "        self.n_actions = n_actions\n",
+    "        self.model = < Your network here>  # Use BBBLinear instead of Linear layers\n",


<YOUR CODE: network. Use BBLinear instead of linear layers>

dniku · 2020-04-14T12:50:09Z

week05_explore/week5.ipynb

@@ -1423,9 +1365,22 @@
  }
 ],
 "metadata": {
+  "kernelspec": {


Please clean up this

dniku · 2020-04-14T12:53:03Z

week05_explore/week5.ipynb

    "\n",
+    "    def predict_sample_rewards(self, states):\n",
+    "        return self.model(torch.Tensor(states))\n",


tensor is lowercase nowadays

Qwasser

Left some comments on codestyle. Will review how it works later.
You should fix environment setup in notebook since it does not require theano installation now.

Qwasser · 2020-04-17T10:47:56Z

week05_explore/bayes.py


-__all__ = ['NormalApproximation', 'get_var_cost', 'bbpwrap']
+class GaussianVariational(nn.Module):
+    #Samples weights for variational inference as in Weights Uncertainity on Neural Networks (Bayes by backprop paper)


Needs space after #
Add link to referred paper.

Qwasser · 2020-04-17T10:55:25Z

week05_explore/bayes.py

+
+
+class ScaleMixturePrior(nn.Module):
+    #Calculates a Scale Mixture Prior distribution for the prior part of the complexity cost on Bayes by Backprop paper


Use ''' doc strings. Add link to paper.

Qwasser · 2020-04-17T10:56:30Z

week05_explore/bayes.py

-        log q(weights|learned mu and rho) aka log q(theta|x)
+        Calculates the log_likelihood for each of the weights sampled relative to a prior distribution as a part of the complexity cost
+        returns:
+            torch.tensor with shape []


what shape?

Qwasser · 2020-04-17T10:57:12Z

week05_explore/bayes.py

-            # if layer is bayesian or pretends so
-            cost += layer.get_var_cost()
-    return cost
+    Bayesian Linear layer, implements the linear layer proposed on Weight Uncertainity on Neural Networks


link to paper

Qwasser · 2020-04-17T10:57:50Z

week05_explore/bayes.py

+                 freeze = False):
+        super().__init__()
+
+        #our main parameters


Spaces after #. Comment is redundatn.

Qwasser · 2020-04-17T10:57:56Z

week05_explore/bayes.py

+        self.bias = bias
+        self.freeze = freeze
+
+        #parameters for the scale mixture prior


Qwasser · 2020-04-17T10:58:16Z

week05_explore/bayes.py

+    def forward(self, x):
+        # Sample the weights and forward it
+
+        #if the model is frozen, return frozen


Comment repeats code. Remove it

Qwasser · 2020-04-17T11:03:20Z

week05_explore/bayes.py

+        # Sample the weights and forward it
+
+        #if the model is frozen, return frozen
+        if self.freeze:


It is boolean variable. Name it is_frozen.

week05_explore/bayes.py

Qwasser

Supply doc string to your layer implementation. It will help students to understand it.
In the notebook sample_prediction method does not work without .detach().numpy()
Is it possible to make 2-layer network no? If not, than "your code here" section in __init__ of BNNAgent is redundant. Can you fill it with model or provide possibility of multilayer agent?
Why Bayesian layer is called BBBLayer?

Qwasser · 2020-04-21T19:04:32Z

week05_explore/bayes.py

-        super().__init__()
-
-        #our main parameters
+    def __init__(self, in_features, out_features, alpha_shape=(1, 1), bias=True, name='BBBLinear'):


Could you write a detailed docstrig here?

dniku · 2020-04-14T18:26:55Z

week05_explore/week5.ipynb

@@ -2,7 +2,7 @@
 "cells": [


Please remove installation of Theano/Lasagne from the Colab initialization cell.

remove theano from week5

59501d4

dniku marked this pull request as draft April 10, 2020 20:08

dniku requested review from dniku and justheuristic April 10, 2020 20:08

dniku assigned yhn112 Apr 10, 2020

dniku added the maintenance label Apr 10, 2020

dniku changed the title ~~Remove theano from week5~~ Remove Theano from Week 5 Apr 10, 2020

dniku requested changes Apr 10, 2020

View reviewed changes

justheuristic requested a review from Qwasser April 11, 2020 17:23

Prefinal version

b4f7b62

dniku reviewed Apr 14, 2020

View reviewed changes

Qwasser reviewed Apr 17, 2020

View reviewed changes

Qwasser reviewed Apr 21, 2020

View reviewed changes

yhn112 linked an issue May 11, 2020 that may be closed by this pull request

Remove Theano/Lasagne support #258

Closed

dniku changed the base branch from spring20 to master August 4, 2020 13:52

dniku reviewed Jan 9, 2021

View reviewed changes

Qwasser marked this pull request as ready for review February 8, 2021 17:06

Merge branch 'master' into week5-remove-theano

5ead383

Qwasser merged commit dab6ae3 into master Feb 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Theano from Week 5 #348

Remove Theano from Week 5 #348

yhn112 commented Apr 10, 2020 •

edited by dniku

Loading

review-notebook-app bot commented Apr 10, 2020

dniku left a comment •

edited by yhn112

Loading

dniku left a comment

dniku Apr 14, 2020

dniku Apr 14, 2020

dniku Apr 14, 2020

dniku Apr 14, 2020

dniku Apr 14, 2020

dniku Apr 14, 2020

dniku Apr 14, 2020

dniku Apr 14, 2020

dniku Apr 14, 2020

Qwasser left a comment

Qwasser Apr 17, 2020

Qwasser Apr 17, 2020

Qwasser Apr 17, 2020

Qwasser Apr 17, 2020

Qwasser Apr 17, 2020

Qwasser Apr 17, 2020

Qwasser Apr 17, 2020

Qwasser Apr 17, 2020

Qwasser left a comment

Qwasser Apr 21, 2020

dniku Apr 14, 2020



		class ScaleMixturePrior(nn.Module):
		#Calculates a Scale Mixture Prior distribution for the prior part of the complexity cost on Bayes by Backprop paper

Remove Theano from Week 5 #348

Remove Theano from Week 5 #348

Conversation

yhn112 commented Apr 10, 2020 • edited by dniku Loading

review-notebook-app bot commented Apr 10, 2020

dniku left a comment • edited by yhn112 Loading

Choose a reason for hiding this comment

dniku left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Qwasser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Qwasser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yhn112 commented Apr 10, 2020 •

edited by dniku

Loading

dniku left a comment •

edited by yhn112

Loading