search

GLSRC / rescomp

Reservoir Computing and related information processing methods
MIT License
5 stars 5 forks source link

small change in _train_synced and _fit_w_out #8

Closed DuncDennis closed 3 years ago

DuncDennis commented 3 years ago

Instead of calculating y_train = x_train[1:] in _train_synced, and parsing it into _fit_w_out, now parse x_train and calculate y_train = x_train[1:] in _fit_w_out. This gives more flexibility for extensions of RC (as Hybrid RC), where w_out is fitted to a concatination of the reservoir states AND another prediction method (which has to be calculated from x_train[:-1]

baurse commented 3 years ago

This seems like a dangerous change. Have you tested this to make sure it doesn't break literally all of the previous code that was written with the old behavior of _fit_w_out in mind?

DuncDennis commented 3 years ago

This seems like a dangerous change. Have you tested this to make sure it doesn't break literally all of the previous code that was written with the old behavior of _fit_w_out in mind?

_ESNCore._fit_w_out is exclusively called in _ESNCore._train_sinced. The argument x_train of _train_synced remains the same in this change as in the previous code, thus, any part of code that uses _train_synced (as for example train) does not have to be changed. Thus, I think this is a safe change.

baurse commented 3 years ago

Yeah you are right, this is definitely a safe merge, as are your other two PRs.

They'll have to wait a bit longer for their inclusion though, as I don't want to merge anything into the master branch anymore w/o finally having a testing setup that takes into account multiple python and package versions. I do have that now and will probably have it finished entirely next week.