mlpack / ensmallen

A header-only C++ library for numerical optimization --
http://ensmallen.org
Other
742 stars 120 forks source link

PrimalDualSolver states arma::mat deprecated type to be removed in 2.10.0, but still present in 2.11.5 #184

Closed jwdinius closed 4 years ago

jwdinius commented 4 years ago

Problem location

https://github.com/mlpack/ensmallen/blob/2.11.5/include/ensmallen_bits/sdp/primal_dual.hpp

Description of problem

The comments below indicate the interface should have been removed by now. (2.11.5 > 2.10.0)

/**
 * PrimalDualSolver is a primal dual interior point solver for semidefinite
 * programs.
 *
 * PrimalDualSolver can optimize semidefinite programs.  For more details, see the
 * documentation on function types included with this distribution or on the
 * ensmallen website.
 *
 * @tparam DeprecatedSDPType Type of SDP to solve.  This parameter is deprecated
 *      and will be removed in ensmallen 2.10.0.
 */
template<typename DeprecatedSDPType = SDP<arma::mat>>

The documentation should be updated to reflect when the interface will actually be deprecated.

birm commented 4 years ago

It looks like we should remove it then.

jwdinius commented 4 years ago

It looks like removal will break the sdp unit tests. Maybe add -Wdeprecated-declarations to catch this type-of thing when building tests?

rcurtin commented 4 years ago

Ack, I think that we failed on this one. Should have opened an issue and tacked it onto a 2.10.0 milestone or something.

Anyway, I think this is a good issue for someone new who's looking to contribute. But, this will break reverse compatibility, so we'll have to release 2.12.0 as a change. Basically the task is to remove the deprecated template parameter to PrimalDualSolver, remove the ens_deprecated methods, and update the tests in tests/ that use it, then ensure that the documentation is still up to date (and all the tests work :)).

jwdinius commented 4 years ago

I'll give it a whack tomorrow or Wednesday. I'm looking to use the interface anyways. :)