scipopt / scip

SCIP - Solving Constraint Integer Programs
Other
369 stars 63 forks source link

IPOPT Linear Solver HSL_MA97 Cause Segmentation Fault #11

Closed chemrgineer closed 2 years ago

chemrgineer commented 2 years ago

Hi, I found bug on SCIP concurrentopt function. I compiled IPOPT with HSL libraries contains HSL_MA97 which use OpenMP for parallel computation. I configure SCIP to use HSL_MA97 via command "set nlpi ipopt linear_solver ma97". When I try to start concurrentopt fails. I think, this problem hard to fix but some explanation can be added for wrong settings. Thanks for for your hard work.

svigerske commented 2 years ago

I had thought that MA97 would be threadsafe. I can try to reproduce, but it might also help if you could just run a debugger and let us know some more details on this segfault (i.e., where does this appear?).

chemrgineer commented 2 years ago

Hi, I found my mistake. I have 5 threads but in default SCIP sets 8. Because of that I get Segmentation fault. Although I corrected maxnthreads option to 4, I get "free(): double free detected in tcache 2" error in debug build.

svigerske commented 2 years ago

So this is how it looks to me:

$ valgrind ./bin/scip -c "read check/instances/MINLP/pointpack06.osil concurrentopt"
==1373718== Memcheck, a memory error detector
==1373718== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1373718== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==1373718== Command: ./bin/scip -c read\ check/instances/MINLP/pointpack06.osil\ concurrentopt
==1373718== 
SCIP version 8.0.0 [precision: 8 byte] [memory: block] [mode: optimized] [LP solver: SoPlex 6.0.0] [GitHash: 083fc00147]
Copyright (C) 2002-2021 Konrad-Zuse-Zentrum fuer Informationstechnik Berlin (ZIB)

External libraries: 
  Readline 8.1         GNU library for command line editing (gnu.org/s/readline)
  SoPlex 6.0.0         Linear Programming Solver developed at Zuse Institute Berlin (soplex.zib.de) [GitHash: b87478dd]
  CppAD 20180000.0     Algorithmic Differentiation of C++ algorithms developed by B. Bell (github.com/coin-or/CppAD)
  ZLIB 1.2.11          General purpose compression library by J. Gailly and M. Adler (zlib.net)
  GMP 6.2.1            GNU Multiple Precision Arithmetic Library developed by T. Granlund (gmplib.org)
  AMPL/MP 4e2d45c4     AMPL .nl file reader library (github.com/ampl/mp)
  PaPILO 2.0.0         parallel presolve for integer and linear optimization (github.com/scipopt/papilo)
  bliss 0.73p          Computing Graph Automorphism Groups by T. Junttila and P. Kaski (www.tcs.hut.fi/Software/bliss/)
  Ipopt 3.14.5         Interior Point Optimizer developed by A. Waechter et.al. (github.com/coin-or/Ipopt)

reading user parameter file <scip.set>

read problem <check/instances/MINLP/pointpack06.osil>
============

original problem has 13 variables (0 bin, 0 int, 0 impl, 13 cont) and 21 constraints

feasible solution found by trivial heuristic after 0.4 seconds, objective value -1.000000e+05
presolving:
(round 1, fast)       0 del vars, 0 del conss, 0 add conss, 1 chg bounds, 0 chg sides, 0 chg coeffs, 0 upgd conss, 0 impls, 0 clqs
(round 2, fast)       0 del vars, 0 del conss, 0 add conss, 2 chg bounds, 0 chg sides, 0 chg coeffs, 0 upgd conss, 0 impls, 0 clqs
   (0.8s) symmetry computation started: requiring (bin +, int +, cont +), (fixed: bin -, int -, cont -)
   (0.9s) no symmetry present
presolving (3 rounds: 3 fast, 1 medium, 1 exhaustive):
 0 deleted vars, 0 deleted constraints, 0 added constraints, 2 tightened bounds, 0 added holes, 0 changed sides, 0 changed coefficients
 0 implications, 0 cliques
presolved problem has 13 variables (0 bin, 0 int, 0 impl, 13 cont) and 21 constraints
      6 constraints of type <linear>
     15 constraints of type <nonlinear>
Presolving Time: 0.68
initializing seeds to 1963210296 in concurrent solver 'scip-2'
initializing seeds to 1332858414 in concurrent solver 'scip-3'
initializing seeds to 1541326760 in concurrent solver 'scip-4'
initializing seeds to 247360965 in concurrent solver 'scip-5'
initializing seeds to 387742462 in concurrent solver 'scip-6'
initializing seeds to 520723434 in concurrent solver 'scip-7'
initializing seeds to 1176648445 in concurrent solver 'scip-8'
transformed 2/2 original solutions to the transformed problem space
starting solve in concurrent solver 'scip-8'
starting solve in concurrent solver 'scip-3'
starting solve in concurrent solver 'scip-6'
starting solve in concurrent solver 'scip-7'
starting solve in concurrent solver 'scip-4'
starting solve in concurrent solver 'scip-2'
starting solve in concurrent solver 'scip-1'
starting solve in concurrent solver 'scip-5'
==1373718== Thread 4:
==1373718== Invalid read of size 4
==1373718==    at 0x76059B2: __hsl_ma97_double_MOD_subtree_factor (hsl_ma97d.f90:3207)
==1373718==    by 0x761027E: __hsl_ma97_double_MOD_ma97_factor_solve_double (hsl_ma97d.f90:2541)
==1373718==    by 0x7611733: __hsl_ma97_double_MOD_ma97_factor_double (hsl_ma97d.f90:2024)
==1373718==    by 0x7629058: ma97_factor_d (hsl_ma97d_ciface.f90:433)
==1373718==    by 0x5CF5FF4: Ipopt::Ma97SolverInterface::MultiSolve(bool, int const*, int const*, int, double*, bool, int) (IpMa97SolverInterface.cpp:706)
==1373718==    by 0x5AFE17D: Ipopt::TSymLinearSolver::MultiSolve(Ipopt::SymMatrix const&, std::vector<Ipopt::SmartPtr<Ipopt::Vector const>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector const> > >&, std::vector<Ipopt::SmartPtr<Ipopt::Vector>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector> > >&, bool, int) (IpTSymLinearSolver.cpp:262)
==1373718==    by 0x5AA4284: Ipopt::StdAugSystemSolver::MultiSolve(Ipopt::SymMatrix const*, double, Ipopt::Vector const*, double, Ipopt::Vector const*, double, Ipopt::Matrix const*, Ipopt::Vector const*, double, Ipopt::Matrix const*, Ipopt::Vector const*, double, std::vector<Ipopt::SmartPtr<Ipopt::Vector const>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector const> > >&, std::vector<Ipopt::SmartPtr<Ipopt::Vector const>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector const> > >&, std::vector<Ipopt::SmartPtr<Ipopt::Vector const>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector const> > >&, std::vector<Ipopt::SmartPtr<Ipopt::Vector const>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector const> > >&, std::vector<Ipopt::SmartPtr<Ipopt::Vector>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector> > >&, std::vector<Ipopt::SmartPtr<Ipopt::Vector>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector> > >&, std::vector<Ipopt::SmartPtr<Ipopt::Vector>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector> > >&, std::vector<Ipopt::SmartPtr<Ipopt::Vector>, std::allocator<Ipopt::SmartPtr<Ipopt::Vector> > >&, bool, int) (IpStdAugSystemSolver.cpp:210)
==1373718==    by 0x572C284: Ipopt::AugSystemSolver::Solve(Ipopt::SymMatrix const*, double, Ipopt::Vector const*, double, Ipopt::Vector const*, double, Ipopt::Matrix const*, Ipopt::Vector const*, double, Ipopt::Matrix const*, Ipopt::Vector const*, double, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector const&, Ipopt::Vector&, Ipopt::Vector&, Ipopt::Vector&, Ipopt::Vector&, bool, int) (IpAugSystemSolver.hpp:104)
==1373718==    by 0x58BFDA2: Ipopt::LeastSquareMultipliers::CalculateMultipliers(Ipopt::Vector&, Ipopt::Vector&) (IpLeastSquareMults.cpp:80)
==1373718==    by 0x5787B55: Ipopt::DefaultIterateInitializer::least_square_mults(Ipopt::Journalist const&, Ipopt::IpoptNLP&, Ipopt::IpoptData&, Ipopt::IpoptCalculatedQuantities&, Ipopt::SmartPtr<Ipopt::EqMultiplierCalculator> const&, double) (IpDefaultIterateInitializer.cpp:701)
==1373718==    by 0x577B861: Ipopt::DefaultIterateInitializer::SetInitialIterates() (IpDefaultIterateInitializer.cpp:340)
==1373718==    by 0x57F3FB4: Ipopt::IpoptAlgorithm::InitializeIterates() (IpIpoptAlg.cpp:655)
==1373718==  Address 0x156688f8 is 40 bytes inside a block of size 64 in arena "client"

So it seems to be indeed a problem with MA97 being not threadsafe (cc @jhogg41). Ipopt already includes a workaround for MUMPS not being threadsafe and something similar could be added to its MA97 interface. I don't think we should change something on the SCIP side.

svigerske commented 2 years ago

I had a closer look and it seems that it is not MA97 being non-threadsafe exactly, but that there is a conflict with the use of OpenMP on the SCIP and MA97 level. If I change SCIP to use TinyCThread instead of OpenMP (TPI=tny) then things seem to work fine. Alternatively, if building HSL without OpenMP (--disable-openmp) then things seem to work fine, too.

Things go wrong in MA97 with memory like buf and stats that exist once per thread. There is code like this:

!$OMP PARALLEL DEFAULT(NONE)                                                  &
!$OMP    PRIVATE(this_thread)                                                 &
!$OMP    SHARED(abort, akeep, buf, control, fkeep, i, info, ldx, map, n,   &
!$OMP       nrhs, stack, stats, val, val2, x)

      this_thread = 1
!$    this_thread = omp_get_thread_num()+1

      allocate(buf(this_thread)%val(akeep%maxmn*128), &
         stat=stats(this_thread)%st)

I guess this doesn't work well if the OpenMP threads were not created by MA97 itself.

I'm not sure whether and where to put a check for this. It may be best if MA97 could check when it starts running that OpenMP is not already "used", whatever that means (omp_get_num_threads()==1?), and then either aborts or runs with one thread only (if that works).
Ipopt doesn't know anything about OpenMP. I don't like it much to change that for something where Ipopt isn't directly involved.
The Ipopt interface of SCIP could be an option. I haven't checked, but if it is possible to figure out whether we are in a concurrent solve with OpenMP, an error could be reported. That would then only be checked in this situation, though, not when there is someone else running multiple threads of SCIP (or something else) via OpenMP.

PS: SCIP nor Ipopt are able to figure out (with reasonable effort) whether MA97 was build with OpenMP, so I don't see a reliable way to report an error anymore.
MA86 also seems to use OpenMP, but works fine under OpenMP-concurrent SCIP. So, well, this seems to be an issue to be handled in MA97.