-
Notifications
You must be signed in to change notification settings - Fork 20
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
There seems something wrong in the implement (group_index). #3
Comments
I also read the Thesis and this code carefully, and found the same question with above, the group_index seems not take effect, the expected effect maybe group_index * group_size? |
here is modified code with group_index * group_size `import numpy as np class ClockworkRNN(object):
|
Hi! Sorry I have not been able to respond earlier. Feel free to make a pull request so I can merge your code into the repository. Thank you! |
@tomrunia , my friend zhlicen already upload the code, please review. and i found another question, will submit another issue. |
this issue appear to be alive after one year... my implementation (based on this repo) is available for reference (together with Temporal Kernel RNN). |
Sorry guys I posted something too fast this morning. It seems to me that this implementation with
is dependent upon the fact that periods are exponents of 2 : 1, 2, 4.. 2^k |
Something else, in this implementation,
Regarding @duducheng's code , his loop over the time sequence (which is built so as to work without the above-mentioned assumption regarding the choice of 2^k for the sequence of periods) makes a lot of useless calculations.
I will try to improve this soon.
|
Here is the bit of code I mentioned yesterday, which makes the model flexible in order to use any periods and not necessarily exponents of 2 and is computationally more efficient : (EVERYTHING inside the " if time_step % self.clockwork_periods[i] == 0:" condition) |
Hi Tom,
I read carefully your code, since I want to implement a network very similar to CWRNN. Your code is really clear, thanks!
However, I found it seems that you didn't correctly implement CWRNN, there is a "little" bug. If you take the
hidden_W
variable value, for instance I take num_hidden=4, periods = [1, 2], and print thehidden_W
change, I got:that's to say your code just use 2 units of
hidden_W
. I think you forget to set thegroup_size
of each period.I think it's better to use tf.where rather than
group_index
, thegroup_index
may cause lots of problem. Check https://github.com/braingineer/ikelos/blob/master/ikelos/layers/cwrnn.py, or later I will give a tensorflow implement.The text was updated successfully, but these errors were encountered: