Hi Ofek, thanks for your comments.
On Aug 31, 2010, at 3:43 AM, Ofek Shilon wrote:
(1) in api/nlopt.h, define NLOPT_EXTERN as follows:
#if defined(NLOPT_DLL) && (defined(_WIN32) || defined(__WIN32__))
# define NLOPT_EXTERN(T) __declspec(dllexport) T NLOPT_STDCALL
#elif (defined(_WIN32) || defined(__WIN32__))
# define NLOPT_EXTERN(T) extern __declspec(dllimport) T NLOPT_STDCALL
#else
# define NLOPT_EXTERN(T) extern T NLOPT_STDCALL
#endif
I don't think this is the best way to do it, because it makes NLopt
impossible to build or use on Windows as a static library. Instead,
following a suggestion by Benoit Scherer, the next release will use
__declspec(dllexport) if you #define NLOPT_DLL_EXPORT when building.
(2) Suggest defining NLOPT_EXTERN_DAT:
#if defined(NLOPT_DLL) && (defined(_WIN32) || defined(__WIN32__))
# define NLOPT_EXPORT_DAT(T) __declspec(dllexport) T
#elif (defined(_WIN32) || defined(__WIN32__))
# define NLOPT_EXPORT_DAT(T) __declspec(dllimport) T
#else
# define NLOPT_EXPORT_DAT(T) extern T
#endif
And decorating nlopt_algorithm_name as follows -
NLOPT_EXPORT_DAT(const char *nlopt_algorithm_name(nlopt_algorithm a));
Ah, thanks for pointing out that I forgot to decorate
nlopt_algorithm_name; I also need to make it stdcall. But why is a
new NLOPT_EXPORT_DAT macro needed? Why can't I just use
NLOPT_EXTERN(const char *) nlopt_algorithm_name(nlopt_algorithm a);
?
(4) in api/nlopt.hpp, suggest modifying mythrow so that it wouldn't
throw every exception *after* a matching case:
I'm not sure I understand what you are trying to accomplish with this
patch. You changed e.g.
case NLOPT_FAILURE: throw std::runtime_error("nlopt failure");
into
case NLOPT_FAILURE: { throw std::runtime_error("nlopt failure");
break; }
Adding the braces does nothing, and the break statement is unnecessary
because the throw statement exits without executing any subsequent
statements anyway.
(6) in DIrect.c, there seems to be some wrong error reporting. Suggest
changing lines 535-540 to -
if (oops) {
if (logfile)
fprintf(logfile, "WARNING: max eval reached in
routine DIRsamplepoints \n");
*ierror = 2; //-4;
goto cleanup;
}
The -4 error code (DIRECT_SAMPLEPOINTS_FAILED) looks correct to me,
not 2 (DIRECT_MAXITER_EXCEEDED). These lines are executed if the
function direct_dirsamplepoints_ has an error, which occurs if the
hard-coded MAXFUNC value is exceeded, not if the user-supplied maximum
evaluation count is exceeded. NLopt reports this as a generic
NLOPT_FAILURE code.
(In the original Fortran direct code, there were all sorts of hard-
coded limits. I relaxed some of these by dynamically allocating the
memory, but there is still a MAXFUNC limit if the user does not
specify a maximum evaluation count. Getting rid of these limitations
was one of the reasons why I wrote my own DIRECT implementation.)
(7) Both luksan\pssubs.c and neldermead\sbplx.c make use of
'copysign()'.
Benoit Scherer pointed out the same problem to me recently; copysign
is a C99 function that MS does not support in the standard fashion.
I've already removed this usage on MS systems (or on Unix systems
where copysign is not present) for the next release.
(8) Suggest defining min(a,b) , max(a,b) and iabs(x) in a central
Benoit also pointed out that MS pollutes the namespace with their own
"max" and "min" macros, so the next release will not use macros with
these names.
I did various other hacks and modifications intended to avoid compiler
warnings (explicit casts and such)
Benoit pointed out several other warnings that I've fixed and will be
in the next release.
Steven
_______________________________________________
NLopt-discuss mailing list
[email protected]
http://ab-initio.mit.edu/cgi-bin/mailman/listinfo/nlopt-discuss
