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.

Sign up for GitHub

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

Better output if the optimization process gets terminated during the execution of an epoch. #180

Closed
wants to merge 6 commits into from
Closed

Better output if the optimization process gets terminated during the execution of an epoch. #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e6b9075
Initial commit.
gaurav-singh1998 Mar 15, 2020
4a02e40
Minor changes.
gaurav-singh1998 Mar 15, 2020
db0b6f9
Style changes.
gaurav-singh1998 Mar 15, 2020
c6fe0dc
Removal of extra progress bar from the output.
gaurav-singh1998 Mar 19, 2020
4561686
Changes in message and progress bar.
gaurav-singh1998 Apr 16, 2020
27be8d5
Minor changes in progress bar.
gaurav-singh1998 Apr 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions include/ensmallen_bits/callbacks/progress_bar.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ class ProgressBar
* @param objective Objective value of the current point.
*/
template<typename OptimizerType, typename FunctionType, typename MatType>
void StepTaken(OptimizerType& /* optimizer */,
FunctionType& /* function */,
void StepTaken(OptimizerType& optimizer,
FunctionType& function,
const MatType& /* coordinates */)
{
if (newEpoch)
Expand Down Expand Up @@ -164,8 +164,43 @@ class ProgressBar
output << "] " << progress << "% - ETA: " << (size_t) stepTimer.toc() *
(epochSize - step + 1) % 60 << "s - loss: " <<
objective / (double) step << "\r";
output.flush();

if (optimizer.MaxIterations() < function.NumFunctions() &&
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that this is the right way to check this condition. This will fail if the termination happens during the second epoch. Here's some example code:

#include <ensmallen.hpp>

int main()
{
  arma::mat data(10, 1000, arma::fill::randu);
  arma::Row<size_t> responses(1000);
  for (size_t i = 0; i < 500; ++i)
    responses[i] = 0;
  for (size_t i = 500; i < 1000; ++i)
    responses[i] = 1;

  ens::test::LogisticRegressionFunction<> lrf(data, responses, 0.1);

  ens::Adam adam;
  adam.BatchSize() = 1;
  adam.MaxIterations() = 1500;
  arma::mat coordinates = arma::randu<arma::mat>(1, 11);
  adam.Optimize(lrf, coordinates, ens::ProgressBar());
}

If you try running that on this branch, you'll see:

$ ./test && echo ""
Epoch 1/2
1000/1000 [====================================================================================================] 100% - 0s 0ms/step - loss: 1.23509
Epoch 2/2
500/1000 [==================================================>.................................................] 50% - ETA: 0s - loss: 0.836281
$

(The && echo "" is necessary because the last \r is printed unnecessarily.)

I wonder if we need to do this in a separate way, where a final call to this callback happens when the method terminates. Perhaps in the destructor of ProgressBar? I'm not sure if that could work, but it might.

optimizer.MaxIterations() != 0)
{
if (progress == (size_t)((double)(optimizer.MaxIterations()) /
(double)(function.NumFunctions()) * 100) &&
(int)(steps * optimizer.BatchSize() -
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, @shrit I made a minor change in this line changing step to steps and got the expected output i.e.

Epoch 1/6
157/1875 [========>...........................................................................................] 8% - ETA: 0s - loss: 2039.62
Optimization terminated because of the entire dataset not being passed to the optimizer.

and now also if ENS_PRINT_INFO is turned on then the output is,

157/1875 [========>...........................................................................................] 8% - ETA: 0s - loss: 2039.62
Optimization terminated because of the entire dataset not being passed to the optimizer.
SGD: maximum iterations (10000) reached; terminating optimization.

Let me know what you think of this approach. Thanks.

optimizer.MaxIterations()) >= 0)
{
const size_t progress = ((double) step / epochSize) * 100;
output << step++ << "/" << epochSize << " [";
for (size_t i = 0; i < 100; i += width)
{
if (i < progress)
{
output << "=";
}
else if (i == progress)
{
output << "|";
}
else
{
output << "x";
}
}

output << "] " << progress << "% - ETA: " << (size_t) stepTimer.toc() *
(epochSize - step + 1) % 60 << "s - loss: " <<
objective / (double) step << "\r";
output << "\n" << "Optimization finished before the end of an epoch "
<< "because of the entire dataset not being passed to "
<< "the optimizer." << "\n" << "\r";
Copy link
Member

Choose a reason for hiding this comment

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

Do you think that we could improve this error message? To a user I don't think it's clear what's going on. I think maybe something more like this could be clearer:

Optimization terminated; maximum iterations reached before the end of an epoch.

Also, why the \r at the end of the line? \n or std::endl should be sufficient. I don't imagine that we are printing anything after termination such that the line needs to be rolled back.

}
}

output.flush();
stepTimer.tic();
}

Expand Down
2 changes: 1 addition & 1 deletion include/ensmallen_bits/cmaes/cmaes.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
namespace ens {

/**
* CMA-ES - Covariance Matrix Adaptation Evolution Strategy is s a stochastic
* CMA-ES - Covariance Matrix Adaptation Evolution Strategy is a stochastic
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch, thanks! 👍

* search algorithm. CMA-ES is a second order approach estimating a positive
* definite matrix within an iterative procedure using the covariance matrix.
*
Expand Down