ANN No copy ctor for FNN

[theSundayProgrammer]

I am trying to modify mlpack::ann::BuildVanillaNetwork to return FFN:
return FFN<decltype(modules), decltype(classOutputLayer), PerformanceFunctionType>(modules, classOutputLayer);
But compilation fails with:
... attempting to reference a deleted function.
Perhaps, one of the items in the tuple does not support copy construction.

[theSundayProgrammer]

I implemented copy ctor. Hence the bug may be closed.

[rcurtin]

Want to file a pull request? Maybe we could incorporate your code.

[theSundayProgrammer]

Sure! I'll let you know when I am ready.

[theSundayProgrammer]

I ran into a circular reference while testing. To be more precise:
LinearLayer contains Optimizer which holds a reference to the container.
Hence simple copy will copy the reference even though the parent object has
changed.

On Fri, Jan 29, 2016 at 2:10 PM, Ryan Curtin notifications@github.com
wrote:

Want to file a pull request? Maybe we could incorporate your code.

—
Reply to this email directly or view it on GitHub
#510 (comment).

Joseph Chakravarti Mariadassou
http://thesundayprogrammer.com

[stereomatchingkiss]

I added move constructor and move assignment on most of the layers in the pull request of #451.
I also change the reference of the output layer of FFN to non-reference type, it would be very easy to implement move constructor and move assignment for the FFN after #451 is merged. Maybe you can copy the codes of the #451 before it is merged, and implement the move assignment and move constructor.

[theSundayProgrammer]

I made the following changes:
(a) removed circular reference in optimizer as in
LinearLayer::LinearLayer: optimizer(*this)
to
`LinearLayer::LinearLayer: optimizer()'

(b) removed Law of Demeter violations as in
t.Optimizer().Reset() to
t.ResetOptimizer()

t.Optimizer().Update() to
t.UpdateOptimizer()

t.Optimizer().Optimize(); to
t.OPtimize()

(c) change optimizer from pointer to shared pointer (TODO: make Optimizer a
member variable)

(d) removed destructors and copy constructors to allow compiler to generate defaults. As there are no raw pointers there is no need for destructor.

(e) Added a few unit tests for copy ctor

(f) All the unit tests except for a couple unrelated to ANN ran to success

The result is in https://github.com/theSundayProgrammer/mlpack

[rcurtin]

Sorry for the slow response, I didn't really have a chance to look into this until today. If you'd like to open a PR, that might be a good idea. A couple quick notes:

• Please take a glance at the style guidelines -- https://github.com/mlpack/mlpack/wiki/StyleGuidelines -- I'm happy to go over it and make changes after a pull, but it saves time if I don't have to :)
• Take a look at the other serialization bits in mlpack, like here: https://github.com/mlpack/mlpack/blob/master/src/mlpack/methods/logistic_regression/logistic_regression_impl.hpp#L143 -- basically, I've implemented a big shim that lets us write Serialize() instead of serialize() for the sake of consistency. Also, you should use data::CreateNVP() to create a name-value pair so that things can successfully be saved to XML archives, instead of just ar & member.

If you open a PR, we can discuss in more detail and probably get @zoq's opinion, which will be the important opinion here.