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

MATLAB interface documentation inconsistency #124

Closed
jan-provaznik opened this issue Dec 30, 2022 · 6 comments
Closed

MATLAB interface documentation inconsistency #124

jan-provaznik opened this issue Dec 30, 2022 · 6 comments

Comments

@jan-provaznik
Copy link
Contributor

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

  • I see a potential issue regarding the definition of converged / target reached within the above code: should we consider the search successful if at least one feasible point was found even though the optimizer eventually ran out of evaluations?
  • I currently do not have access to MATLAB-compatible version of GCC. Once I do, and if desired, I can eventually turn this into a pull request.
  • Essentially identical code could be used to resolve Matlab and python interface exit status #104.
@ctribes
Copy link
Contributor

ctribes commented Jan 9, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor

ctribes commented Jan 9, 2023

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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor

ctribes commented Jan 11, 2023

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)

@ctribes ctribes closed this as completed Jan 9, 2024
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

No branches or pull requests

3 participants