neuroevolution-ai / NeuroEvolution-CTRNN_new

MIT License
3 stars 3 forks source link

transfer input normalization into iBrain #47

Closed Dranero closed 3 years ago

Dranero commented 3 years ago

Hello, since the config option normalize_input and the method _normalize_input are in the IBrain class, i would recomend to also shift the lines

if self.config.normalize_input:
    ob = self._normalize_input(ob, self.input_space, self.config.normalize_input_target)

of each implemented network in this class and therefore make the step method to an template method pattern, rather than an abstract method.

pdeubel commented 3 years ago

Sounds good to me this would also refactor things like discretizing the observations etc.

One problem is that each brain does something else in the step function after the input is pre-processed. Maybe use the step() function of IBrain as the pre-processor and call it in every subclassed method with super.step() as the first thing, or create a new mandatory function for each subclass which implements their logic for the original step() function and then call it in the template method in IBrain after the pre-processing. So that the input is processed and then fed into each brains individual logic. I think the second option would be better.

bjuergens commented 3 years ago

related: https://github.com/neuroevolution-ai/NeuroEvolution-CTRNN_new/issues/13

bjuergens commented 3 years ago

implemented in #49 iirc

pdeubel commented 3 years ago

Not yet merged but I removed the normalization in #53.