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

Remove TensorVariable Init #549

Merged
merged 1 commit into from
Dec 31, 2015
Merged

Remove TensorVariable Init #549

merged 1 commit into from
Dec 31, 2015

Conversation

skaae
Copy link
Member

@skaae skaae commented Dec 23, 2015

Removes TensorVariable special case as discussed here: #462

  • Removes TensorVariable Init.
  • Removes related Tests

@skaae
Copy link
Member Author

skaae commented Dec 23, 2015

I noticed that hid_init does not have a reference to lasagne.layers.Layer .
:class:lasagne.layers.Layer does not fit the linewidth but :class:Layer` does.

Should I change add :class:Layer and maybe also change the rest of the recurrent docs to use
:class:Layer because its more consistent with the rest of the docs?

@f0k
Copy link
Member

f0k commented Dec 29, 2015

Removes TensorVariable special case

Looks good!

Should I change add :class:Layer and maybe also change the rest of the recurrent docs to use
:class:Layer because its more consistent with the rest of the docs?

Yes, would be good to change that! :class:Layer is enough because we tell Sphinx that we're in the lasagne.layers namespace. The docstring change should be in a separate commit this time, i.e., not squashed into the first one. Let me know when you're done so we can merge! (github doesn't send out a notification for additional commits to a PR, only for additional comments.)

@skaae
Copy link
Member Author

skaae commented Dec 29, 2015

I added references to the layer class in the doc string and squashed the commits.

Initializer for initial hidden state (:math:`h_0`). If a
TensorVariable (Theano expression) is supplied, it will not be learned
regardless of the value of `learn_init`.
cell_init : callable, np.ndarray, theano.shared or Layer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you missed that one!

@f0k
Copy link
Member

f0k commented Dec 29, 2015

One Layer reference missed, other than that, it's good to merge. Can you please fix that (and squash)? Thanks!

@skaae
Copy link
Member Author

skaae commented Dec 29, 2015

Sorry about that. I fixed it.
Edit: And squashed.

@f0k
Copy link
Member

f0k commented Dec 31, 2015

Thanks, merging!

f0k added a commit that referenced this pull request Dec 31, 2015
@f0k f0k merged commit 5520c06 into Lasagne:master Dec 31, 2015
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.

None yet

2 participants