bbopt / nomad

NOMAD - A blackbox optimization software
https://nomad-4-user-guide.readthedocs.io/
GNU Lesser General Public License v3.0
117 stars 24 forks source link

MATLAB interface documentation inconsistency #124

Closed jan-provaznik closed 9 months ago

jan-provaznik commented 1 year ago

The documentation for the MATLAB interface diverges from the actual implementation.

  1. According to the documentation, nomadOpt returns [ x, fval, exitflag, iter, nfval ], however, the actual implementation returns [ x, fval, hinf, exitflag, nfval ]. The example reflects the actual implementation.
  2. Values of the exitflag do not follow the documented scheme. Instead of the declared spectrum of return values, only two values are returned and their meaning differs from the documentation. The value 0 represents generic success, whereas -1 indicates no feasible result was found.

I believe it may be possible to modify nomadmex.cpp to approach the documented behavior. The following pseudo-code might do that.

bool hasConverged = (nbBestFeas > 0);
bool hasInfeasible = (nbBestInf > 0);

bool exitInitializationError = AllStopReasons::testIf(INITIALIZATION_FAILED);
bool exitUser = (
  AllStopReasons::testIf(BaseStopType::CTRL_C) ||
  AllStopReasons::testIf(BaseStopType::USER_STOPPED));
bool exitNomadError = (
  AllStopReasons::testIf(BaseStopType::ERROR) ||
  AllStopReasons::testIf(BaseStopType::UNKNOWN_STOP_REASON));
bool exitExceededEvaluations =  (
  AllStopReasons::testIf(EvalGlobalStopType::MAX_EVAL_REACHED) ||
  AllStopReasons::testIf(EvalGlobalStopType::MAX_BB_EVAL_REACHED) ||
  AllStopReasons::testIf(EvalGlobalStopType::MAX_BLOCK_EVAL_REACHED) ||
  AllStopReasons::testIf(EvalGlobalStopType::MAX_SURROGATE_EVAL_OPTIMIZATION_REACHED));

if (exitUser) {
  * exitflag = -5;
}
else if (exitNomadError) {
  * exitflag = -3;
}
else if (exitInitializationError) {
  * exitflag = -2;
}
else if (hasConverged) {
  * exitflag = 1;
}
else if (exitExceededEvaluations) {
  * exitflag = 0;
}
else if (hasInfeasible) {
  * exitflag = -1;
}
else {
  // Something else must have happened.
  // There are neither feasible nor infeasible points and there is no clear reason for termination. 
  // Let's chalk that up as 'nomad error'.
  * exitflag = -3;
}

REMARKS

ctribes commented 1 year ago

Thanks for reporting this inconsistency. I guess this happened when porting from Nomad 3.9 to Nomad 4. The same piece of code can be used for Python and Matlab interfaces. I update the issue #104 to make sure I will handle both at the same time.

jan-provaznik commented 1 year ago

If we come to some agreement about the interpretation of exit states I can implement the feature for the Python interface first and make a pull request with the changes. Sounds like fun to me.

ctribes commented 1 year ago

Ideally, for consistency, the exist status should be returned by a MainStep function (like NOMAD::MainStep::getExitStatus, to be implemented) determined from the _stopReasons attribute. This information can be used to return the exist status by nomad.exe, matlab interface (nomadOpt function) and PyNomad interface (PyNomad.optimize),....

Maybe Nomad tools should return an exist status with the same logic as linux command. Zero (0) as a success (feasible optimal solution, that is min mesh size reached OR target reached). 1-255 are for failures. I need to do a little bit of thinking about that.

Arrowstar commented 1 year ago

Maybe Nomad tools should return an exist status with the same logic as linux command. Zero (0) as a success (feasible optimal solution, that is min mesh size reached OR target reached). 1-255 are for failures. I need to do a little bit of thinking about that.

I would advise against this. The MATLAB convention is that 1 is successful, other positive integers are "success with caveats" and negative exit flags represent failure. You may confuse a bunch of users with the approach you outlined.

jan-provaznik commented 1 year ago

Maybe Nomad tools should return an exist status with the same logic as linux command. Zero (0) as a success (feasible optimal solution, that is min mesh size reached OR target reached). 1-255 are for failures. I need to do a little bit of thinking about that.

I would advise against this. The MATLAB convention is that 1 is successful, other positive integers are "success with caveats" and negative exit flags represent failure. You may confuse a bunch of users with the approach you outlined.

I wanted to write a similar comment, that it is generally better to herald success with values such that bool(value) == true, especially if the bool cast appears implicitly in if (value). Then I read what @ctribes wrote again and realized that extension of the current interface was proposed.

The bindings for MATLAB or Python could realistically return a [ success, exit_status, ..., best_feasible_fval, best_feasible_xval, ... ] where success would determine if everything went swimmingly and exit_status could be a detailed error (caveat) report adhering to UNIX-like return value logic.

ctribes commented 1 year ago

Based on the previous comments, I have made some changes in PR #125. I changed the term "exit_status" to "run_flag" to prevent confusion with Unix-like return codes. Both Matlab and Python interfaces return the same run flags using the MainStep getRunFlag function.

Run flags: % Run flags: % 1 - Objective target reached OR Mads converged (mesh criterion) to a feasible point (true problem). % 0 - At least one feasible point obtained and evaluation budget (single bb or block of bb) spent or max iteration (user option) reached. % -1 - Mads mesh converged but no feasible point obtained (only infeasible) for the true problem. % -2 - No feasible point obtained (only infeasible) and evaluation budget (single bb or block of bb) spent or max iteration (user option) reached % -3 - Initial point failed to evaluate % -4 - Time limit reached (user option) % -5 - CTRL-C or user stopped (callback function) % -6 - Stop on feasible point (user option)