Rewrite this part

[zaikunzhang]

First of all, explain what success, terminate, and direction_indices mean in the following piece of comment: https://github.com/blockwise-direct-search/bds/blob/93121ff9fc188932ddb6b443110fc141fe0f8e5f/src/private/inner_direct_search.m#LL21C1-L25C21

% [XVAL, FVAL, EXITFLAG, OUTPUT] = INNER_DIRECT_SEARCH(...) returns a
% structure OUTPUT with the number of function evaluations in OUTPUT.funcCount,
% the history of function evaluation in OUTPUT.fhist, the history of points
% in OUTPUT.xhist, boolean value of success, boolean value of terminate and
% direction_indices.

Secondly, in the following code,

https://github.com/blockwise-direct-search/bds/blob/9b8f0d99a6b0266f2e9cf763d2d2518c22b71188/src/private/inner_direct_search.m#LL109C1-L112C62

I think

    % In the opportunistic case, if the current iteration achieves the 
    % sufficient decrease,  stop the computations after cycling the 
    % indices of the polling directions.
    if sufficient_decrease && ~strcmpi(options.polling_inner, "complete")

sounds more logical better than

    % In the opportunistic case, if the current iteration is successful,
    % stop the computations after cycling the indices of the polling
    % directions.
     if success && ~strcmpi(options.polling_inner, "complete")

The code is equivalent. What do you think @ragonneau ? (@Lht97 Make the changes if Tom does not say no)

[ragonneau]

I agree with you @zaikunzhang. Yesterday, we saw that naming the variable success led to confusion, and sufficient_decrease is self-explanatory.

@Lht97 Many programming conventions accept the fact that it is even possible to not comment on a variable if its name is explicit and does not lead to any confusion (for example Google C++ style). If your variable is success, you obviously need to explain in detail (success of what?).

Cheers, Tom.

[zaikunzhang]

Secondly, in the following code,

https://github.com/blockwise-direct-search/bds/blob/9b8f0d99a6b0266f2e9cf763d2d2518c22b71188/src/private/inner_direct_search.m#LL109C1-L112C62

I think

% In the opportunistic case, if the current iteration achieves the % sufficient decrease, stop the computations after cycling the % indices of the polling directions. if sufficient_decrease && ~strcmpi(options.polling_inner, "complete") sounds more logical better than

% In the opportunistic case, if the current iteration is successful, % stop the computations after cycling the indices of the polling % directions. if success && ~strcmpi(options.polling_inner, "complete") The code is equivalent. What do you think @ragonneau ? (@Lht97 Make the changes if Tom does not say no)

Why do you @Lht97 close the issue without making the change?

https://github.com/blockwise-direct-search/bds/blob/463c431e5351d7f0785323127a88acac1779c4d9/src/private/inner_direct_search.m#L127

Did you forget it or did you not understand it?

[zaikunzhang]

@Lht97 Did you make this change?

[Lht97]

Write it more detailed and logically.

[Lht97]

Write comments of those variables more detailed. Explain them logically and describe different situations of them.

[Lht97]

Explain the meanings and implications of each variable. How is it initialized, and how is it updated?

[Lht97]

I have explained the meaning of success terminate, and direction_indices detailed. (https://github.com/blockwise-direct-search/bds/blob/00e99a8454ad3a0bf0facb078128b117cd33925a/src/private/inner_direct_search.m#L22) Also, comment on when to cycle the direction_indices is better now (https://github.com/blockwise-direct-search/bds/blob/00e99a8454ad3a0bf0facb078128b117cd33925a/src/private/inner_direct_search.m#L152)