coin-or / Ipopt

COIN-OR Interior Point Optimizer IPOPT
https://coin-or.github.io/Ipopt
Other
1.45k stars 285 forks source link

Corrupt sol-file from AMPL app in Ipopt 3.13 #369

Closed eslickj closed 4 years ago

eslickj commented 4 years ago

I'm getting corrupt sol files out of the AMPL app for Ipopt 3.13. I don't think I'm the only one, see https://github.com/Pyomo/pyomo/issues/1239. I was able to fix the problem by using the AMPL app code from 3.12 with 3.13, so the problem is most likely in one of those files.

EDIT: Seems I was mistaken the thing that seems to be working is to edit the dependencies to use the 1.4 version of the ASL build tools, but I doing some more testing to be sure that really works.

svigerske commented 4 years ago

I couldn't reproduce this with the .nl file from the Pyomo issue. I have a build of Ipopt stable/3.12 with --enable-debug on Linux with gcc 9.3.0, but I get very tiny numbers (like 1.83670851354919e-40) where they report to have a p printed in the .sol file. The tiny number seems to be correct. tmpriak07qo.pyomo.sol

Further, when printing solution values that are exactly 0, I see valgrind complain:

==2441244== Conditional jump or move depends on uninitialised value(s)
==2441244==    at 0x4C0C5F4: x_sprintf (printf.c:747)
==2441244==    by 0x4C0E31C: Sprintf (printf.c:1143)
==2441244==    by 0x4BCB415: g_fmtp_ASL (g_fmt.c:49)
==2441244==    by 0x4C2FD32: write_solfx_ASL (writesol.c:436)
==2441244==    by 0x4C3041D: write_sol_ASL (writesol.c:527)
==2441244==    by 0x486EE6E: Ipopt::AmplTNLP::write_solution_file(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (AmplTNLP.cpp:952)
==2441244==    by 0x486E9CD: Ipopt::AmplTNLP::finalize_solution(Ipopt::SolverReturn, int, double const*, double const*, double const*, int, double const*, double const*, double, Ipopt::IpoptData const*, Ipopt::IpoptCalculatedQuantities*) (AmplTNLP.cpp:833)
==2441244==    by 0x494A17C: Ipopt::TNLPAdapter::FinalizeSolution(Ipopt::SolverReturn, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, double, Ipopt::IpoptData const*, Ipopt::IpoptCalculatedQuantities*) (IpTNLPAdapter.cpp:2201)
==2441244==    by 0x4A20A92: Ipopt::OrigIpoptNLP::FinalizeSolution(Ipopt::SolverReturn, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, double, Ipopt::IpoptData const*, Ipopt::IpoptCalculatedQuantities*) (IpOrigIpoptNLP.cpp:948)
==2441244==    by 0x492E432: Ipopt::IpoptApplication::call_optimize() (IpIpoptApplication.cpp:1193)
==2441244==    by 0x492B017: Ipopt::IpoptApplication::OptimizeNLP(Ipopt::SmartPtr<Ipopt::NLP> const&, Ipopt::SmartPtr<Ipopt::AlgorithmBuilder>&) (IpIpoptApplication.cpp:806)
==2441244==    by 0x492ACC1: Ipopt::IpoptApplication::OptimizeNLP(Ipopt::SmartPtr<Ipopt::NLP> const&) (IpIpoptApplication.cpp:762)
==2441244==  Uninitialised value was created by a stack allocation
==2441244==    at 0x4C0AF70: x_sprintf (printf.c:393)
==2441244== 

But I get the same with Ipopt from stable/3.12 when using the same ASL library (build via current master of https://github.com/coin-or-tools/ThirdParty-ASL)

Are you sure you only replaced the source files in src/Apps/AmplSolver by the older version?

eslickj commented 4 years ago

Yeah. I did, and it somehow fixed the Windows build. Testing with Linux builds though, showed that it wasn't really fixed. That's really strange, but now I don't think that's where the problem is anymore.

The thing that now seems to work for everything is to just switch the ASL build dependency to 1.4. I'm still testing on a few platforms to be sure it really fixes the issue everywhere.

eslickj commented 4 years ago

I've done quite a bit of testing now, and I think I confirmed that using the old ASL app didn't fix anything, but it did somehow change the way the problem shows up somehow. In testing whit Pyomo I've seen all of the following. Switching the ASL build tools version to 1.4 does seems to fix everything.

1) UTF-8 decoding error reading solve file 2) Could not convert string to float, some random non-numeric characters. 3) Results that are slighty wrong.

I guess it must be related to the uninitialized value thing.

svigerske commented 4 years ago

https://github.com/coin-or-tools/ThirdParty-ASL master and stable/2.0 uses a complete different way to build the ASL sources. Of course I tried to restore most of the compiler and linker flag that were used originally and https://github.com/coin-or-tools/ThirdParty-ASL/blob/stable/1.4/get.ASL#L53 made me believe that we've always build with -DNo_dtoa. This disables the use of ASLs own float-to-string conversion. So I added this to the compiler flags for the new buildsystem, too: https://github.com/coin-or-tools/ThirdParty-ASL/blob/785008dd0114411675cf1be9195b7e27cef2e135/configure.ac#L33

Having a closer look, I see that the -DNo_dtoa does actually not make it to the compiler call. The CFLAGS get overwritten sometime later again. Removing the -DNo_dtoa from the compiler flags in the new buildsystem makes the valgrind complain go away for me. I guess there is some bug in the No_dtoa-version of ASL's printf-implementation.

I'll remove adding -DNo_dtoa in master of coin-or-tools/ThirdParty-ASL and maybe you can test that again. I hope things will still compile on other platforms if we use dtoa again.

PS: We also updated to the most recent ASL version, but that didn't seem to have an effect.

eslickj commented 4 years ago

Thanks. I just tested with the ThridParty-ASL master branch, and it seems to be working right for me.

svigerske commented 4 years ago

Great, thanks. I've updated stable/2.0 of ThirdParty/ASL.