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

Basic DQN #1014

Merged
merged 14 commits into from Jun 16, 2017
2 changes: 1 addition & 1 deletion COPYRIGHT.txt
Expand Up @@ -37,7 +37,7 @@ Copyright:
Copyright 2014, Udit Saxena <saxenda.udit@gmail.com>
Copyright 2014-2015, Stephen Tu <tu.stephenl@gmail.com>
Copyright 2014-2015, Jaskaran Singh <jaskaranvirdi@ymail.com>
Copyright 2015, Shangtong Zhang <zhangshangtong.cpp@icloud.com>
Copyright 2015&2017, Shangtong Zhang <zhangshangtong.cpp@gmail.com>
Copyright 2015, Hritik Jain <hritik.jain.cse13@itbhu.ac.in>
Copyright 2015, Vladimir Glazachev <glazachev.vladimir@gmail.com>
Copyright 2015, QiaoAn Chen <kazenoyumechen@gmail.com>
Expand Down
37 changes: 26 additions & 11 deletions src/mlpack/methods/ann/ffn.hpp
Expand Up @@ -51,15 +51,18 @@ class FFN

/**
* Create the FFN object with the given predictors and responses set (this is
* the set that is used to train the network) and the given optimizer.
* the set that is used to train the network).
* Optionally, specify which initialize rule and performance function should
* be used.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @param outputLayer Output layer used to evaluate the network.
* @param initializeRule Optional instantiated InitializationRule object
* for initializing the network parameter.
*/
FFN(OutputLayerType&& outputLayer = OutputLayerType(),
FFN(OutputLayerType outputLayer = OutputLayerType(),
InitializationRuleType initializeRule = InitializationRuleType());

//! Copy constructor.
Expand All @@ -73,19 +76,22 @@ class FFN

/**
* Create the FFN object with the given predictors and responses set (this is
* the set that is used to train the network) and the given optimizer.
* the set that is used to train the network).
* Optionally, specify which initialize rule and performance function should
* be used.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @param predictors Input training variables.
* @param responses Outputs results from input training variables.
* @param outputLayer Output layer used to evaluate the network.
* @param initializeRule Optional instantiated InitializationRule object
* for initializing the network parameter.
*/
FFN(const arma::mat& predictors,
const arma::mat& responses,
OutputLayerType&& outputLayer = OutputLayerType(),
FFN(arma::mat predictors,
arma::mat responses,
OutputLayerType outputLayer = OutputLayerType(),
InitializationRuleType initializeRule = InitializationRuleType());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a question more for @zoq: what is the intended use of this constructor and the version that takes const arma::mat&s? I don't see it used in this code, and I'm not sure why a user would want to initialize the network with some data but not train it. Maybe there is some reason I haven't thought of. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I can't think of any situation where that might be necessary. Maybe I just looked at the LinearRegression/LogisticRegression class and thought it's a good idea.


//! Destructor to release allocated memory.
Expand All @@ -99,6 +105,9 @@ class FFN
* optimization. If this is not what you want, then you should access the
* parameters vector directly with Parameters() and modify it as desired.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @tparam OptimizerType Type of optimizer to use to train the model.
* @param predictors Input training variables.
* @param responses Outputs results from input training variables.
Expand All @@ -109,8 +118,8 @@ class FFN
mlpack::optimization::RMSProp,
typename... OptimizerTypeArgs
>
void Train(const arma::mat& predictors,
const arma::mat& responses,
void Train(arma::mat predictors,
arma::mat responses,
OptimizerType<NetworkType, OptimizerTypeArgs...>& optimizer);

/**
Expand All @@ -122,24 +131,30 @@ class FFN
* optimization. If this is not what you want, then you should access the
* parameters vector directly with Parameters() and modify it as desired.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @tparam OptimizerType Type of optimizer to use to train the model.
* @param predictors Input training variables.
* @param responses Outputs results from input training variables.
*/
template<
template<typename...> class OptimizerType = mlpack::optimization::RMSProp
>
void Train(const arma::mat& predictors, const arma::mat& responses);
void Train(arma::mat predictors, arma::mat responses);

/**
* Predict the responses to a given set of predictors. The responses will
* reflect the output of the given output layer as returned by the
* output layer function.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @param predictors Input predictors.
* @param results Matrix to put output predictions of responses into.
*/
void Predict(const arma::mat& predictors, arma::mat& results);
void Predict(arma::mat predictors, arma::mat& results);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original Predict function is fairly inconvenient to use especially when I want to preserve the predictors. So I just add a wrapper.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good for me, I think we should use a reference here, there might be compilers which are not going to optimize this way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But returning a non-const reference to a local variable is an undefined behavior. And I think NRVO is fairly reliable unless conditional return.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was talking about predictors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. The reason why I didn't use arma::mat Predict(arma::mat& predictors); is that I want predictors to stay unchanged after calling Predict. So I want a copy here. If the signature of the original function is void Predict(const arma::mat& predictors, arma::mat& results);, I won't write this wrapper. But I find it's not so easy to change the original function to const reference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok that makes sense. Then I think what we need to do here is to implement void Predict(mat&&, mat&) directly and implement void Predict(const mat&, mat&) as a wrapper. (Perhaps need to revert the fix).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good plan for me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So will you do this or I make it in this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you like you can do here, don't feel obligated, just let me know.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I can do it here

/**
* Evaluate the feedforward network with the given parameters. This function
Expand Down Expand Up @@ -226,7 +241,7 @@ class FFN
* @param predictors Input data variables.
* @param responses Outputs results from input data variables.
*/
void ResetData(const arma::mat& predictors, const arma::mat& responses);
void ResetData(arma::mat predictors, arma::mat responses);

/**
* The Backward algorithm (part of the Forward-Backward algorithm). Computes
Expand Down
42 changes: 21 additions & 21 deletions src/mlpack/methods/ann/ffn_impl.hpp
Expand Up @@ -29,9 +29,9 @@ namespace ann /** Artificial Neural Network. */ {

template<typename OutputLayerType, typename InitializationRuleType>
FFN<OutputLayerType, InitializationRuleType>::FFN(
OutputLayerType&& outputLayer, InitializationRuleType initializeRule) :
OutputLayerType outputLayer, InitializationRuleType initializeRule) :
outputLayer(std::move(outputLayer)),
initializeRule(initializeRule),
initializeRule(std::move(initializeRule)),
width(0),
height(0),
reset(false)
Expand All @@ -41,22 +41,20 @@ FFN<OutputLayerType, InitializationRuleType>::FFN(

template<typename OutputLayerType, typename InitializationRuleType>
FFN<OutputLayerType, InitializationRuleType>::FFN(
const arma::mat& predictors,
const arma::mat& responses,
OutputLayerType&& outputLayer,
arma::mat predictors,
arma::mat responses,
OutputLayerType outputLayer,
InitializationRuleType initializeRule) :
outputLayer(std::move(outputLayer)),
initializeRule(initializeRule),
initializeRule(std::move(initializeRule)),
width(0),
height(0),
reset(false)
reset(false),
predictors(std::move(predictors)),
responses(std::move(responses)),
deterministic(true)
{
numFunctions = responses.n_cols;

this->predictors = std::move(predictors);
this->responses = std::move(responses);

this->deterministic = true;
numFunctions = this->responses.n_cols;
}

template<typename OutputLayerType, typename InitializationRuleType>
Expand All @@ -68,7 +66,7 @@ FFN<OutputLayerType, InitializationRuleType>::~FFN()

template<typename OutputLayerType, typename InitializationRuleType>
void FFN<OutputLayerType, InitializationRuleType>::ResetData(
const arma::mat& predictors, const arma::mat& responses)
arma::mat predictors, arma::mat responses)
{
numFunctions = responses.n_cols;
this->predictors = std::move(predictors);
Expand All @@ -88,11 +86,11 @@ template<
typename... OptimizerTypeArgs
>
void FFN<OutputLayerType, InitializationRuleType>::Train(
const arma::mat& predictors,
const arma::mat& responses,
arma::mat predictors,
arma::mat responses,
OptimizerType<NetworkType, OptimizerTypeArgs...>& optimizer)
{
ResetData(predictors, responses);
ResetData(std::move(predictors), std::move(responses));

// Train the model.
Timer::Start("ffn_optimization");
Expand All @@ -106,7 +104,7 @@ void FFN<OutputLayerType, InitializationRuleType>::Train(
template<typename OutputLayerType, typename InitializationRuleType>
template<template<typename...> class OptimizerType>
void FFN<OutputLayerType, InitializationRuleType>::Train(
const arma::mat& predictors, const arma::mat& responses)
arma::mat predictors, arma::mat responses)
{
numFunctions = responses.n_cols;

Expand Down Expand Up @@ -134,7 +132,7 @@ void FFN<OutputLayerType, InitializationRuleType>::Train(

template<typename OutputLayerType, typename InitializationRuleType>
void FFN<OutputLayerType, InitializationRuleType>::Predict(
const arma::mat& predictors, arma::mat& results)
arma::mat predictors, arma::mat& results)
{
if (parameter.is_empty())
{
Expand All @@ -148,7 +146,8 @@ void FFN<OutputLayerType, InitializationRuleType>::Predict(
}

arma::mat resultsTemp;
Forward(std::move(predictors.col(0)));
Forward(std::move(arma::mat(predictors.colptr(0),
predictors.n_rows, 1, false, true)));
resultsTemp = boost::apply_visitor(outputParameterVisitor,
network.back()).col(0);

Expand All @@ -157,7 +156,8 @@ void FFN<OutputLayerType, InitializationRuleType>::Predict(

for (size_t i = 1; i < predictors.n_cols; i++)
{
Forward(std::move(predictors.col(i)));
Forward(std::move(arma::mat(predictors.colptr(i),
predictors.n_rows, 1, false, true)));

resultsTemp = boost::apply_visitor(outputParameterVisitor,
network.back());
Expand Down
31 changes: 23 additions & 8 deletions src/mlpack/methods/ann/rnn.hpp
Expand Up @@ -46,10 +46,13 @@ class RNN

/**
* Create the RNN object with the given predictors and responses set (this is
* the set that is used to train the network) and the given optimizer.
* the set that is used to train the network).
* Optionally, specify which initialize rule and performance function should
* be used.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @param rho Maximum number of steps to backpropagate through time (BPTT).
* @param single Predict only the last element of the input sequence.
* @param outputLayer Output layer used to evaluate the network.
Expand All @@ -63,10 +66,13 @@ class RNN

/**
* Create the RNN object with the given predictors and responses set (this is
* the set that is used to train the network) and the given optimizer.
* the set that is used to train the network).
* Optionally, specify which initialize rule and performance function should
* be used.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @param predictors Input training variables.
* @param responses Outputs results from input training variables.
* @param rho Maximum number of steps to backpropagate through time (BPTT).
Expand All @@ -75,8 +81,8 @@ class RNN
* @param initializeRule Optional instantiated InitializationRule object
* for initializing the network parameter.
*/
RNN(const arma::mat& predictors,
const arma::mat& responses,
RNN(arma::mat predictors,
arma::mat responses,
const size_t rho,
const bool single = false,
OutputLayerType outputLayer = OutputLayerType(),
Expand All @@ -93,6 +99,9 @@ class RNN
* optimization. If this is not what you want, then you should access the
* parameters vector directly with Parameters() and modify it as desired.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @tparam OptimizerType Type of optimizer to use to train the model.
* @param predictors Input training variables.
* @param responses Outputs results from input training variables.
Expand All @@ -103,8 +112,8 @@ class RNN
mlpack::optimization::StandardSGD,
typename... OptimizerTypeArgs
>
void Train(const arma::mat& predictors,
const arma::mat& responses,
void Train(arma::mat predictors,
arma::mat responses,
OptimizerType<NetworkType, OptimizerTypeArgs...>& optimizer);

/**
Expand All @@ -116,6 +125,9 @@ class RNN
* optimization. If this is not what you want, then you should access the
* parameters vector directly with Parameters() and modify it as desired.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @tparam OptimizerType Type of optimizer to use to train the model.
* @param predictors Input training variables.
* @param responses Outputs results from input training variables.
Expand All @@ -124,17 +136,20 @@ class RNN
template<typename...> class OptimizerType =
mlpack::optimization::StandardSGD
>
void Train(const arma::mat& predictors, const arma::mat& responses);
void Train(arma::mat predictors, arma::mat responses);

/**
* Predict the responses to a given set of predictors. The responses will
* reflect the output of the given output layer as returned by the
* output layer function.
*
* If you want to pass in a parameter and discard the original parameter
* object, be sure to use std::move to avoid unnecessary copy.
*
* @param predictors Input predictors.
* @param results Matrix to put output predictions of responses into.
*/
void Predict(const arma::mat& predictors, arma::mat& results);
void Predict(arma::mat predictors, arma::mat& results);

/**
* Evaluate the recurrent neural network with the given parameters. This
Expand Down