4 ms·
I am the author of this work. I was not aware of the difference in the licenses and just used the GitHub default. I have updated it accordingly, thanks for poin
by markpkcollier 8y ago
I am the author of this work. I was not aware of the difference in the licenses and just used the GitHub default. I have updated it accordingly, thanks for pointing this out.
A note on the difference between our work and Snowkylin's: the code implementing the operations of the NTM are very similar and both are similar to other open source NTM implementations as the operations for a NTM are defined by the equations of the original NTM paper and have a clear mapping into code.
The primary difference is that our implementation works and is stable - the code changes to achieve this are minimal but this still required substantial experimentation and work to figure out what was causing slow convergence and the gradients becoming NaN (causing training to fail) in other implementations. Thus our primary contribution is not to put a NTM into code but to get that code to reliably and quickly train.
- orbifold 8y agoJust to be clear, I went through the two implementations (https://github.com/snowkylin/ntm/blob/master/ntm/ntm_cell.py https://github.com/snowkylin/ntm/blob/master/ntm/ntm_cell.py) and (https://github.com/MarkPKCollier/NeuralTuringMachine/blob/master/ntm.py https://github.com/MarkPKCollier/NeuralTuringMachine/blob/ma...) line by line. The code is not just similar, but largely identical. You've used the exact same variable names and ordering in the code. Some comments have been removed. The notable differences I can spot are: - line 53-56 (in your code), which correspond to line 41-45 in Snowkylin's code - line 70 vs 69, where you've used a build in function instead of the explicit compositional form of softplus - finally the major change I can see are various variable initialisation schemes 147-178 in your code, which is handled by line 157-185 in snowlkylin's code and some miscellaneous places where you've used a NamedTuple instead of a dictionary. And yes the choice of initialisation is a valid improvement. As far as I can tell Snowkylin's implementation is not broken, but just might learn more slowly because of a different state initialisation scheme. It does not suffer the NaN problem for example. It should be communicated very clearly also in the paper that the code you are releasing is derived from another open-source implementation, with a precise explanation of the changes you have made. Also you can't change the LICENSE and copyright notice of open-source code to yourself without permission. I believe even in the current form your Github repo is in breach of the LGPL3. For more details how to use the LGPL properly, please refer to https://www.gnu.org/licenses/gpl-howto.en.html https://www.gnu.org/licenses/gpl-howto.en.html. Notice that the original repository actually does not carry out all those steps properly. Namely it is missing LICENSE and Copyright notices in every file. It also doesn't have an explicit copyright notice anywhere.
- andreyk 8y agoIndeed, if these are so similar your repo really ought to be a fork of the prior repo, and you should certainly acknowledge them more in the paper (make clearer what your contributions are).
- qwert7890 8y agoYou broke the LGPL license, you didn't state changes: https://github.com/snowkylin/ntm/blob/master/LICENSE https://github.com/snowkylin/ntm/blob/master/LICENSE Moreover, in the paper 5 times you write: "Our implementation" You also don't acknowledge that you are piggybacking on snowkylin's code. You didn't make it magically "stable", like fixed NaNs or anything like that. And you want to be cited as follows: title={Implementing Neural Turing Machines, author={Collier, Mark and Beel, Joeran}, That's just very bad. You should: 1. State changes (orbifold did it for you below). Acknowledge snowkylin. Link to them. 2. Title it "Improved initialization in NTM" or something like that, not "Implementing NTM".