PaPILO icon indicating copy to clipboard operation
PaPILO copied to clipboard

Switch to 64-bit indices

Open mlubin opened this issue 4 years ago • 4 comments

Closes #41

Tests are passing with Soplex and SCIP. I didn't actually try an instance that exceeds 2 billion variables/constraints.

TODOs:

  • [ ] The interface with HiGHS should be tested. It's probably broken.
  • [x] Is the HitCount typedef in Sparsify.hpp safe as an int16?

Before merging, I recommend:

  • Checking the changes very carefully
  • Running benchmarks (are there standard ones?) to check if there's a performance loss in common case where 32-bit integers would be sufficient

Minor:

  • The code calling LUSOL might not need to make an extra copy of the matrix anymore.

mlubin avatar Dec 14 '20 05:12 mlubin

The hit count is safe as rows are not tried when they exceed the types length

lgottwald avatar Dec 17 '20 13:12 lgottwald

The hit count is safe as rows are not tried when they exceed the types length

Thanks for clarifying. I will test HiGHS next, although given the holidays, it may be a week or so before I get to it.

mlubin avatar Dec 22 '20 02:12 mlubin

I'm not able to test HiGHS because it appears that PaPILO isn't compatible with HiGHS master:

PaPILO/src/papilo/interfaces/HighsInterface.hpp:84:22: error: use of undeclared identifier 'OBJSENSE_MINIMIZE'
      model.sense_ = OBJSENSE_MINIMIZE;
papilo/interfaces/HighsInterface.hpp:89:13: error: no member named 'nnz_' in 'HighsLp'
      model.nnz_ = consMatrix.getNnz();

Nevertheless I think the PR is ready to be reviewed.

mlubin avatar Dec 31 '20 03:12 mlubin

I already looked at the changes and it looks ok. The HiGHS interface needs updating independently of this anyways as some things on the HiGHS side changed since I wrote the interface. Currently performance tests of this change are being conducted and once they show that this does not hurt performance significantly we can merge this.

lgottwald avatar Jan 07 '21 13:01 lgottwald