libprima / prima

PRIMA is a package for solving general nonlinear optimization problems without using derivatives. It provides the reference implementation for Powell's derivative-free optimization methods, i.e., COBYLA, UOBYQA, NEWUOA, BOBYQA, and LINCOA. PRIMA means Reference Implementation for Powell's methods with Modernization and Amelioration, P for Powell.
http://libprima.net
BSD 3-Clause "New" or "Revised" License
290 stars 35 forks source link

Fix https://github.com/libprima/prima/issues/193 #196

Open zaikunzhang opened 2 months ago

zaikunzhang commented 2 months ago

This includes the fixes proposed by @amontoison in https://github.com/libprima/prima/pull/194 to fix https://github.com/libprima/prima/issues/193

zaikunzhang commented 2 months ago

Thank you @amontoison for your approval.

Hi @nbelakovski ,

I hope to hear your opinion before merging, especially on the following lines regarding prima_rc_t.

https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L124-L125 (cl complains that PRIMA_RESULT_INITIALIZED is not covered explicitly by a case)

https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L152-L153 (message was set to NULL, which was suggested by me, but I now think an explicit message is better, more consistent with status, and also safer in case message is printed)

https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L196-L199 (info was prima_rc_t, which would cause a warning of incompatible type when &info is passed to xyz_c)

https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L237-L238 (It was return PRIMA_INVALID_INPUT; I prefer not to return in the middle; the revised version seems more consistent)

https://github.com/libprima/prima/blob/fix_issue_193/c/prima.c#L241-L245 (It was prima_get_rc_string(info) and return info)

Thank you.

zaikunzhang commented 2 months ago

Hi @amontoison and @nbelakovski ,

In case no other solution is proposed, where do you think we should place prima_internal.h? I am not familiar with C conventions, but I thought include/prima was only for the public header. What do others do? I suppose we are not the first to face this situation.

Thanks.

Zaikun

amontoison commented 2 months ago

Hi @amontoison and @nbelakovski ,

In case no other solution is proposed, where do you think we should place prima_internal.h? I am not familiar with C conventions, but I thought include/prima was only for the public header. What do others do? I suppose we are not the first to face this situation.

Thanks.

Zaikun

@zaikunzhang It can stay in include/prima but you just don't install it in ${prefix} folder after the compilation.

zaikunzhang commented 2 months ago

@zaikunzhang It can stay in include/prima but you just don't install it in ${prefix} folder after the compilation.

Thank you @amontoison . This would necessitate changing some CMakeList.txt, right? Could you show me how to do it? I have no basic idea bout CMake. Thanks.

amontoison commented 2 months ago

@zaikunzhang It seems that you don't need to do something. Only prima.h is installed: https://github.com/libprima/prima/blob/main/c/CMakeLists.txt#L32

zaikunzhang commented 2 months ago

Thank you @amontoison for your approval.

Hi @nbelakovski ,

I hope to hear your opinion before merging, especially on the following lines regarding prima_rc_t.

fix_issue_193/c/prima.c#L124-L125 (cl complains that PRIMA_RESULT_INITIALIZED is not covered explicitly by a case)

fix_issue_193/c/prima.c#L152-L153 (message was set to NULL, which was suggested by me, but I now think an explicit message is better, more consistent with status, and also safer in case message is printed)

fix_issue_193/c/prima.c#L196-L199 (info was prima_rc_t, which would cause a warning of incompatible type when &info is passed to xyz_c)

fix_issue_193/c/prima.c#L237-L238 (It was return PRIMA_INVALID_INPUT; I prefer not to return in the middle; the revised version seems more consistent)

fix_issue_193/c/prima.c#L241-L245 (It was prima_get_rc_string(info) and return info)

Thank you.

Hi @nbelakovski , do these changes look good to you? Thank you.