search

axa-group / nlp.js

An NLP library for building bots, with entity extraction, sentiment analysis, automatic language identify, and so more
MIT License
6.28k stars 621 forks source link

Is this an issue in package netural-network? #447

Closed taoyuan closed 1 year ago

taoyuan commented 4 years ago

Here k is not initialize. https://github.com/axa-group/nlp.js/blob/38745194f88b88a2cac32e374dbb37f48e11e79c/packages/neural/src/neural-network.js#L243-L247

If init k with 0

 for (let k = 0; k < incoming.length; k += 1) { 
   const change = momentum * changes[k]; 
   changes[k] = change; 
   weights[k] += change; 
 }

the below testing will be failed.

https://github.com/axa-group/nlp.js/blob/38745194f88b88a2cac32e374dbb37f48e11e79c/packages/neural/test/neural-network.test.js#L108-L118

jesus-seijas-sp commented 4 years ago

Hello!

First at all, good catch. Yes, is an issue, it should be initialized so what is wrong are the tests.

Now the curious part: as k is undefined, the loop is not executed. I'm testing with my corpus in different languages, and the scale of the difference is at 1/1000:

image

It has some sense, as this loop is only executed when there is no error at the perceptron, it achieved error 0, but we add a little delta because of the momentum (no spread of the error is done as there is no error).

Basically, don't having this loop improves the time performance, but having this loop is more correct from an algebraic stand point... Right now I don't know if remove this part (better time perfomance) or correct it (mathematically correct).

aigloss commented 1 year ago

Closing due to inactivity