Options

[jschueller]

Move options to prima_options struct that gets initialized to default values so we only need to set those different than defaults, we can see the actual c call takes much fewer args:

  prima_options options;
  prima_init_options(&options);
  options.iprint = PRIMA_MSG_EXIT;
  options.rhoend= 1e-3;
  options.maxfun = 200*n;
  prima_results results;
  const int rc = prima_uobyqa(&fun, n, x, &options, &results);
  printf("nf=%d f=%g\n", results.nf, results.f);
  prima_free_options(&options);
  prima_free_result(&results);

Also it will be easier to add/remove options without changing the api

[zaikunzhang]

Hi Julien @jschueller ,

Thank you very much for this proposal, which is a very good idea.

I hope Éric and Alexis @emmt @amontoison could provide some comments, given that PRIMA.jl is the only major project known to us using the C binding for the moment. In addition, it would be nice if Tom @ragonneau could also help me to check. He is familiar with the algorithms and the Fortran implementation. My knowledge of C is outdated and I have some other urgent deadlines to meet until the end of the month.

BTW, you have been invited to join the libprima GitHub organisation. I also sent you a message at schueller@phimeca.com. Thank you for taking a look when convenient.

Merci beaucoup à tous ! Bon week-end ! (Par hasard, tout le monde mentionné ci-dessus parle français :))

A+, Zaikun

[zaikunzhang]

Comments copied from https://github.com/libprima/PRIMA.jl/issues/14#issuecomment-1773484943 :

The Fortran implementation of PRIMA did not use any "structure" (the name is "derived type" in Fortran) for the input, output, or anywhere. This is because I wanted to provide a reference implementation that exposes the algorithms without using any language-specific features. In addition, I was afraid that using derived types as input or output may introduce problems when interfacing with other languages.

In contrast, the wrappers/interfaces/implementations of PRIMA in other languages should use structures (or something equivalent in the respective language) whenever this leads to better code than otherwise.

[jschueller]

yes, I was also wondering whether I should create a struct for the output and I think I should do it because that would allow to almost unify all the solvers with the same api call

[zaikunzhang]

Quick comments

1. Will we deal with other options like eta1/2, gamma1/2?

2. About maxfun=500*n. This 500 is defined in the Fortran code as MAXFUN_DIM_DFT in consts.F90. Is it possible to use this constant instead of literal number?

Thank you.

[jschueller]

1. yes, it will be easier to use new options without changing the api
2. I dont think I can use fortran constants from C, but i'll check

[zaikunzhang]

2. I dont think I can use fortran constants from C, but i'll check

Understandable. Then it may be a good idea to define macros like PRIMA_MSG_EXIT instead of using literal numbers. The consistency of the macros with the Fortran constants may be checked manually (of course, we may also write some code to check such consistency as part of the building if desired). Thanks.

[jschueller]

good idea, done

[jschueller]

I dont know it seemed natural but I have no strong opinion, what do you prefer, singular or plural ?

[emmt]

I would say plural. But it is really a detail. Sorry for bothering you with this.

[jschueller]

done

[jschueller]

@zaikunzhang I think its good to go

[zaikunzhang]

Thank you @jschueller for the great efforts, and @emmt , @amontoison for checking.

I am currently too busy, so I can only give quick comments but cannot respond quickly.

The code is of high quality, as always.

I have comments related to previous comments from @emmt .

Perhaps arguments defining the constraints (bounds, linear and non-linear constraints) could be kept outside this structure as it is clearly where the different algorithms differ the most. If you keep all inputs in the same structure, algorithms may verify that they are suitable to the given constraints.

I agree with this one. Indeed, if we think about it twice, we will find that the input consists of two distinct groups of data, one group defining the problem, and another defining how to solve the problem (options). They are of different natures and it is unreasonable to mix them.

There is another intrinsic difference between problem and options. By definition, the word options indicates that the data inside are optional. Users may provide them or not, which should not change the nature of the problem being solved, but only affect how the problem will be solved. The data in the problem, however, are not optional --- they may be empty or none, but that is how the problem is defined; if the user does not provide a certain part of the data, then the problem is not completely defined.

If we want to put things into structures to have compact input, then we should have two: problem and options. The problem consists of f, x0 (yes, the starting point is part of the problem), xl, xu, Aineq, bineq, Aeq, beq, and the nonlinear constraints (if it is not together with f). The options consists of rhobeg, rhoend, .... Another possibility is to have only options, but it should not include the data that define the problem.

I hope the API will be changed not so often. So I raise this for your consideration.

Why have you chosen options (plural) and result (singular)?

IMHO, this is the correct choice. As mentioned above, options means parameters that the user can optionally set / choose, so it should be plural. result, however, is unique. It is what we obtain by running the solver. It may contain multiple components, but it is unique.

Maybe scipy.optimize.minimize is a good reference: https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.minimize.html : options and OptimizeResult.

But of course, I am not a native speaker. My opinion may be wrong.

Thanks.

[jschueller]

and ftarget ? part of problem I assume ?

[zaikunzhang]

and ftarget ? part of problem I assume ?

It feels to me like an option. It is part of the stopping criteria. Setting it to a larger value will not change the mathematical problem being solved, but the user chooses to stop earlier.

[zaikunzhang]

x0 (yes, the starting point is part of the problem)

x0 is indeed quite special. Is it optional? Mathematically maybe. But in the case of PRIMA, we need the user to provide x0, mainly because it tells us the shape of the decision variable --- mathematically, the shape information is part of f, but most programming languages do not work in that way.

Another (bad) argument is that, for local optimization algorithms like those in PRIMA, changing the starting point effectively changes the problem. So x0 is part of the problem. Of course, this is a very bad argument.

Thanks.

[jschueller]

ok, added prima_problem too

[zaikunzhang]

Thank @jschueller for the very prompt responses and the new implementation.

Do you @emmt @amontoison @ragonneau have time to check the new implementation? If no, I will check it this weekend.

Thanks to all!

[jschueller]

It think we can go even further after that and choose the algorithm from a enum (int) instead of the 5 functions:

enum prima_algorithm = {PRIMA_COBYLA, PRIMA_BOBYQA, ...}
int prima_minimize(int algorithm, prima_problem *problem, prima_options *options, prima_result *result);

[nbelakovski]

I've looked through the PR and I haven't found anything wrong with it. I looked at what options are exposed to C and validated that everything that was already exposed to C is still exposed to C with this change (i.e. I don't see that anything has fallen through the cracks while making the changes). Additionally I see that f0 and nlconstr0 in COBYLA are exposed to C with this PR (solving issue #99).

I also see that the tests under data.c have run and have passed. There are tests under stress.c that have not run since they are not configured to be run automatically for a PR. @zaikunzhang can you trigger those tests? I don't think I have permission to do so, but if you'd like to grant the permission I can take care of it. It's not clear to me if they exercise code paths that are not already exercised by the data.c tests, but I think it would not hurt to run them and maybe they do exercise some not-otherwise-exercised paths.

[zaikunzhang]

Many thanks to @jschueller and all the reviewers! Everything is great! I am sorry for my slow response.