Skip to content

Conversation

@ranjodhsingh1729
Copy link

@ranjodhsingh1729 ranjodhsingh1729 commented Oct 19, 2025

Description

This PR fixes the following issue in multiple optimizers:

Example

If maxIterations is non-zero it will perform one less iteration.

for (size_t i = 1; i != maxIterations && !terminate; ++i)

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

Thanks; this is a really minor fix but I definitely don't mind adding it. There is a consistency issue worth thinking about here: there are a number of optimizers that use code exactly like this (before your fix), and there are also some that don't:

$ grep -r 'for (size_t i = 1' ~/src/ensmallen/include/
/home/ryan/src/ensmallen/include/ensmallen_bits/gradient_descent/gradient_descent_impl.hpp:  for (size_t i = 1; i != maxIterations && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/fw/frank_wolfe_impl.hpp:  for (size_t i = 1; i != maxIterations && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/fw/constr_structure_group.hpp:    for (size_t i = 1; i <= nGroups; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/fista/fista_impl.hpp:  for (size_t i = 1; i != maxIterations && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/parallel_sgd/parallel_sgd_impl.hpp:  for (size_t i = 1; i != maxIterations && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/nsga2/nsga2_impl.hpp:    for (size_t i = 1; i < fSize - 1; i++)
/home/ryan/src/ensmallen/include/ensmallen_bits/iqn/iqn_impl.hpp:  for (size_t i = 1; i != maxIterations && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/cne/cne_impl.hpp:  for (size_t i = 1; i < populationSize; i++)
/home/ryan/src/ensmallen/include/ensmallen_bits/cmaes/active_cmaes_impl.hpp:  for (size_t i = 1; (i != maxIterations) && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/cmaes/cmaes_impl.hpp:  for (size_t i = 1; (i != maxIterations) && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/fasta/fasta_impl.hpp:  for (size_t i = 1; i != maxIterations && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/cd/cd_impl.hpp:  for (size_t i = 1; i != maxIterations && !terminate; ++i)
/home/ryan/src/ensmallen/include/ensmallen_bits/fbs/fbs_impl.hpp:  for (size_t i = 1; i != maxIterations && !terminate; ++i)

So it would make sense to fix all of them, but also, there is the inconsistency of iteration number: this gradient descent optimizer will print "iteration 1" on the first iteration, but, e.g., BigBatchSGD will print "iteration 0" on its first iteration. I think it would be easiest to just set the first iteration to 0 instead of 1.

Anyway, up to you whether you want to fix just this one or the other ones I grepped for too. Either way is an improvement. 👍

@ranjodhsingh1729 ranjodhsingh1729 changed the title Fix GradientDescent iterations off by one issue Fix an off by one issue affecting number of iterations in multiple optimizers. Oct 20, 2025
@ranjodhsingh1729
Copy link
Author

Anyway, up to you whether you want to fix just this one or the other ones I grepped for too. Either way is an improvement.

No issue! I'll add the others.

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

Successfully merging this pull request may close these issues.

2 participants