Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialization of ann #513

Closed
stereomatchingkiss opened this issue Jan 31, 2016 · 19 comments
Closed

Serialization of ann #513

stereomatchingkiss opened this issue Jan 31, 2016 · 19 comments

Comments

@stereomatchingkiss
Copy link
Contributor

I would like to make the ann support serialization(atleast for the ffn related parts).The optimizers of ann use reference to keep the address of the object, could I change them to pointer or shared_pointer?

@zoq
Copy link
Member

zoq commented Jan 31, 2016

If you do this, we can't create a default optimizer in the way we do it right now, Because in C++ 'this' (keyword) is not an lvalue, so it isn't possible to create a pointer to this. Correct me if I'm on the wrong track.

@stereomatchingkiss
Copy link
Contributor Author

According to the answer of stackoverflow, it should be safe to that as long as we do not access the member haven't construct yet(including virtual member function, but mlpack do not use any virtual, so I think this would not be an issue).

What I mean is, change the reference to pointer as following

RMSPROP(DecomposableFunctionType* function,
          const double lr = 0.01,
          const double alpha = 0.99,
          const double eps = 1e-8) :
      function(function),
      lr(lr),
      alpha(alpha),
      eps(eps)

So we could construct it as

SparseBiasLayer(const size_t outSize,
                  const size_t batchSize,
                  WeightInitRule weightInitRule = WeightInitRule()) :
      outSize(outSize),
      batchSize(batchSize),
      optimizer(new OptimizerType<SparseBiasLayer<OptimizerType,
                                                  WeightInitRule,
                                                  InputDataType,
                                                  OutputDataType>,
                                                  InputDataType>(this)),
      ownsOptimizer(true)


Is this the problem you are worrying about?Or we are talking different problems?

ps : I forgot I open the same issue on #495, rcurtin has an example. If you think it is ok, I would find a way to serialize it.

@zoq
Copy link
Member

zoq commented Feb 1, 2016

I see that would solve the issue, you are right. If you make the changes, can you also modify the other ann optimizer classes?

I guess it would be a good idea, to close one of the pull requests and make a reference to the other pull request, what do you think?

@rcurtin
Copy link
Member

rcurtin commented Feb 1, 2016

One of the keys of the refactoring I did for serialization is that I attempted to keep the interfaces using references, and not pointers, for the sake of simplicity. i.e.

RMSPROP(DecomposableFunctionType* function,
          const double lr = 0.01,
          const double alpha = 0.99,
          const double eps = 1e-8) :
      function(&function),
      lr(lr),
      alpha(alpha),
      eps(eps)

I guess this would make the call to new OptimizerType<> take *this instead of this.

@stereomatchingkiss
Copy link
Contributor Author

can you also modify the other ann optimizer classes?

No problem

I attempted to keep the interfaces using references, and not pointers

Nice suggestion, but I think what you mean is

RMSPROP(DecomposableFunctionType& function, //reference, not pointer
          const double lr = 0.01,
          const double alpha = 0.99,
          const double eps = 1e-8) :
      function(&function),
      lr(lr),
      alpha(alpha),
      eps(eps)

Thanks for closing replicated issue

@rcurtin
Copy link
Member

rcurtin commented Feb 1, 2016

Yeah, exactly, I am just not very good at typing :)

@stereomatchingkiss
Copy link
Contributor Author

Some problems need to be solved.

Optimizer of ann will take the address of DecomposableFunctionType, the solution I come up is

void Serialize(Archive& ar, const unsigned int /* version */)
  {
    using mlpack::data::CreateNVP;

    ar & CreateNVP(outSize, "outSize");
    ar & CreateNVP(bias, "bias");
    ar & CreateNVP(weights, "weights");
    ar & CreateNVP(delta, "delta");
    ar & CreateNVP(gradient, "gradient");
    ar & CreateNVP(inputParameter, "inputParameter");
    ar & CreateNVP(outputParameter, "outputParameter");
    ar & CreateNVP(optimizer, "optimizer");    
    ar & CreateNVP(ownsOptimizer, "ownsOptimizer");

    //after serialize optimizer, 
    optimizer.DecomposableFunction(*this);
  }

optimizer part

template<typename Archive>
  void Serialize(Archive& ar, const unsigned int /* version */)
  {
    using mlpack::data::CreateNVP;

    //do not serialize function, the address of the function
    //should be setup by the DecomposableFunction
    //ar & CreateNVP(function, "function");
    ar & CreateNVP(lr, "lr");
    ar & CreateNVP(alpha, "alpha");
    ar & CreateNVP(eps, "eps");
    ar & CreateNVP(meanSquaredGad, "meanSquaredGad");
    ar & CreateNVP(gradient, "gradient");
  }

What do you think about it?Thanks

@zoq
Copy link
Member

zoq commented Feb 3, 2016

Looks good to me. I would change the DecomposableFunction(...) function to Function(...), what do you think?

@stereomatchingkiss
Copy link
Contributor Author

Looks good to me. I would change the DecomposableFunction(...) function to Function(...), what do you think?

Looks good to me

@stereomatchingkiss
Copy link
Contributor Author

One more problem, CreateNVP cannot serialize arma::Cube, mlpack do not implement this feature yet or I do not use it properly? Thanks

ps : armadillo 6.5 provide conv2 for 2D convolution, maybe this can simplify and speed up the convolution layer? Besides, serialization of optimizer and most of the layers are done(except of conv_layer, lstm_layer and pooling_layer, because they need to serialize arma::cube)

@rcurtin
Copy link
Member

rcurtin commented Feb 4, 2016

I haven't implemented serialization of cubes yet. Take a look at src/mlpack/core/arma_extend/Mat_extra_bones.hpp, src/mlpack/core/arma_extend/Mat_extra_meat.hpp, and src/mlpack/core/arma_extend/arma_extend.hpp to see how I wrote serialize() for arma::Mat<>. You can do the same thing with cubes, just the definitions will be ARMA_EXTRA_CUBE_PROTO and ARMA_EXTRA_CUBE_MEAT. Are you willing to implement that for cube? It shouldn't be too hard, but if you do, can you also write a few tests in serialization_test.cpp? (You can look at the tests I wrote for Mat there too.)

If you don't have time, I can get around to it eventually, but as always, my todo list is quite long... :)

@stereomatchingkiss
Copy link
Contributor Author

I will implement serialization of cubes and write the test cases, looks like it would take a while before FFN can be serialize(at first I though it should be quite fast)

@stereomatchingkiss
Copy link
Contributor Author

I have some questions about the file Mat_extra_meat.hpp, the function

void Mat<eT>::serialize(Archive& ar, const unsigned int /* version */)

Looks like a member function of the Mat of armadillo, how could you add it into the declaration of armadillo?Thanks

@rcurtin
Copy link
Member

rcurtin commented Feb 8, 2016

Ooh, this is a really cool trick that Conrad wrote into Armadillo and I really like it. Inside of Mat_bones.hpp there are these lines, inside of the definition of the class Mat:

  public:

  #ifdef ARMA_EXTRA_MAT_PROTO
    #include ARMA_INCFILE_WRAP(ARMA_EXTRA_MAT_PROTO)
  #endif

So if you've defined ARMA_EXTRA_MAT_PROTO, it just gets included right there in the class definition. The same is true in Mat_meat.hpp, and a few other files (including the cube files). This is really cool because it allows the class to be extensible. So I add the definitions of the functions in Mat_extra_bones.hpp and set that file to be ARMA_EXTRA_MAT_PROTO, then the implementations in Mat_extra_meat.hpp. I think it's a really cool trick. :)

@stereomatchingkiss
Copy link
Contributor Author

@rcurtin

Thanks for the guide, I implement the serialization of arma::Cube and provide a simple test on #520

@theSundayProgrammer
Copy link
Contributor

Serialization is not complete AFAIK: The following test fails (100% error):

  auto net = CreateVanillaNetwork();
  RMSprop<decltype(net)> opt(net, 0.01, 0.88, 1e-8, 10 * input.n_slices, 0);

  net.Train(input, Y, opt);
  data::Save("LearntMachine.xml", "ConvNetwork", net, mlpack::data::xml );
  size_t error = 0;
  {
    arma::mat prediction;
    net.Predict(input, prediction);
    ...
    }
  }
  auto net2 = CopyVanillaNetwork();
  data::Load("LearntMachine.xml", "ConvNetwork", net2, mlpack::data::xml);
  {
    arma::mat prediction;
    net2.Predict(input, prediction);
    ...
}

Note that net computes the values as expected. But net2 fails with 100% error. This is partly because the weights have not been serialized. But adding network and outputLayer to CNN::Serialize did not help.

@zoq
Copy link
Member

zoq commented Mar 4, 2016

I guess, the problem can be solved by using something like this:

Serialize(Archive& ar, const unsigned int /* version */)
{
    ar & data::CreateNVP(parameter, "parameter");

    if (Archive::is_loading::value)
    {
        NetworkWeights(parameter, network);
    }
}

I can't test this right now.

@zoq
Copy link
Member

zoq commented Mar 9, 2016

In c75652b I made the necessary changes, let me know if that fixes your error.

@zoq zoq closed this as completed Mar 9, 2016
@theSundayProgrammer
Copy link
Contributor

Thanks. I "smoke" tested CNN serialization and it works.

On Thu, Mar 10, 2016 at 3:56 AM, Marcus Edel notifications@github.com
wrote:

Closed #513 #513.


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

Joseph Chakravarti Mariadassou
http://thesundayprogrammer.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants