jeffheaton / encog-java-core

http://www.heatonresearch.com/encog
Other
742 stars 268 forks source link

org.encog.ml.ea.opp.CompoundOperator not Serializable #241

Closed dsmaugy closed 7 years ago

dsmaugy commented 7 years ago

I'm trying to serialize my BasicEA object which implements Serializable, but I get this exception: java.io.NotSerializableException: org.encog.ml.ea.opp.CompoundOperator Is it safe to just go into the source and make that class serializable?

xerx593 commented 7 years ago

Hi, This sounds like an approach! ...and when the next exception fires, you'll have to drill down (deeper in the structure of your BasicEA). ... to serialize one class/object, all of its (non-transient) "descendants" must be/implement serializable.

Best regards,

Alex

Am 21.02.2017 8:04 PM schrieb "GandalfD" notifications@github.com:

I'm trying to serialize my BasicEA object which implements Serializable, but I get this exception: java.io.NotSerializableException: org.encog.ml.ea.opp.CompoundOperator Is it safe to just go into the source and make that class serializable?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/encog/encog-java-core/issues/241, or mute the thread https://github.com/notifications/unsubscribe-auth/AAR_npsixoqf-n7b09JggUJR1W4fb0TXks5rezUjgaJpZM4MHun7 .

dsmaugy commented 7 years ago

Hmm, I did this but I kept having to serialize classes until the stack trace said that I needed to serialize Thread. Would setting the non serializable instance variables of BasicEA to transient break the BasicEA?

dsmaugy commented 7 years ago

Well I tried making the ThreadPoolExecutor transient, and I'm getting a null pointer exception when the BasicEA tries to load up threads

xerx593 commented 7 years ago

No, I really(!) doubt, that it would break something of the current functionality (with full tests you are (ever) on a safer side), but next questionable would be the "quality/requirements" of (de-)serialisation.

Kind regards,

Alex

Am 21.02.2017 9:27 PM schrieb "GandalfD" notifications@github.com:

Hmm, I did this but I kept having to serialize classes until the stack trace said that I needed to serialize Thread. Would setting the non serializable instance variables of BasicEA to transient break the BasicEA?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/encog/encog-java-core/issues/241#issuecomment-281469925, or mute the thread https://github.com/notifications/unsubscribe-auth/AAR_noSilrWc9pZp8U16UJpx33l0rl_eks5re0jEgaJpZM4MHun7 .

xerx593 commented 7 years ago

ThreadpoolExecutor ... NPE, huh? :( Do you have a (full) stacktrace maybe, and if you fork it .. I would take a look.

Am 21.02.2017 10:38 PM schrieb "Alexander Lutz" alexlutz79@gmail.com:

No, I really(!) doubt, that it would break something of the current functionality (with full tests you are (ever) on a safer side), but next questionable would be the "quality/requirements" of (de-)serialisation.

Kind regards,

Alex

Am 21.02.2017 9:27 PM schrieb "GandalfD" notifications@github.com:

Hmm, I did this but I kept having to serialize classes until the stack trace said that I needed to serialize Thread. Would setting the non serializable instance variables of BasicEA to transient break the BasicEA?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/encog/encog-java-core/issues/241#issuecomment-281469925, or mute the thread https://github.com/notifications/unsubscribe-auth/AAR_noSilrWc9pZp8U16UJpx33l0rl_eks5re0jEgaJpZM4MHun7 .

dsmaugy commented 7 years ago

I actually think I fixed it?

In the BasicEA class, I changed this check in the iteration() method.

if (this.actualThreadCount == -1) {
    this.preIteration();
} 

to

if (this.actualThreadCount == -1 || this.taskExecutor == null) {
    this.preIteration();
}

I didn't seem to get any errors so I guess that is working. Maybe it's a placebo, but I feel like my training runs a lot slower now. Maybe somehow I ruined multithreading?

jeffheaton commented 7 years ago

The difficulty in serializing the trainers is that is that some of the members are not serializable, mostly the multithreading components. I think the best approach would be to let the threading components go to null and then detect/recreate them as needed after the trainer is reloaded. I never really persisted the actual trainers in my own code, What is the use case? I assume you want to save them so that you can resume training later?

On Tue, Feb 21, 2017 at 9:29 PM, GandalfD notifications@github.com wrote:

I actually think I fixed it?

In the BasicEA class, I changed this check in the iteration() method. if (this.actualThreadCount == -1) { this.preIteration(); } to if (this.actualThreadCount == -1 || this.taskExecutor == null) { this.preIteration(); }

I didn't seem to get any errors so I guess that is working. Maybe it's a placebo, but I feel like my training runs a lot slower now. Maybe somehow I ruined multithreading?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/encog/encog-java-core/issues/241#issuecomment-281558559, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcPqfjB-R906hMeTlSn78hkqFyrai66ks5re6uwgaJpZM4MHun7 .

dsmaugy commented 7 years ago

I assume you want to save them so that you can resume training later?

Yes, I want to be able to train for a couple of iterations, stop the training program if needed, and then resume training again with the same best genome.

I thought that just serializing the NEATPopulation would suffice, but everytime I restarted the program, the training object wouldn't train to beat the best genome of the NEATPopulation, it would act as if it was training with fresh training data.

As I was typing this, I just thought to myself, would manually setting the best genome of the train object to the best genome of the population fix this and negate the need for serialization of the training object?

jeffheaton commented 7 years ago

It is safe enough for that member variable, but to make it fully serializable a few other adjustments are needed. I am going to check in a change for that soon. Your fix might work, but I would have to try it to know for sure. I think serializing the population is the cleaner solution than serializing the trainer. But I will have a look at that.

jeffheaton commented 7 years ago

I was able to reproduce this issue and checked in a fix that allows you to serialize an EATrain object being used with NEAT. This is checked into the current codebase and will be in the next release. You can see a unit test that demonstrates persisting a trainer and reloading it:

https://github.com/encog/encog-java-core/blob/master/src/test/java/org/encog/persist/TestPersistNEAT.java

jeffheaton commented 7 years ago

Fixed as of 3.4