coin-or / CoinUtils

COIN-OR Utilities
Other
46 stars 42 forks source link

factor starting at position 1025 interpreted as variable name #147

Closed dlaehnemann closed 3 years ago

dlaehnemann commented 3 years ago

After the latest input parsing update, my larger problem file now turns up this problem:

Welcome to the CBC MILP Solver 
Version: 2.10.5 
Build Date: Feb 19 2021 

command line - /path/to/bin/cbc factor_starting_at_1025.lp solve (default strategy 1)
 CoinLpIO::readLp(): Maximization problem reformulated as minimization
### CoinLpIO::is_invalid_name(): Name 1183 should not start with a number
### CoinLpIO::are_invalid_names(): Invalid name: vnames[59]: 1183
### CoinLpIO::readLp(): Invalid column names
Now using default column names.

A minimal example to reproduce this is: factor_starting_at_1025.txt

But for more extensive input testing, I'll just include the larger input file that I have been extracting all these cases from. The model itself doesn't actually make much sense in that form, but the format should be alright. So maybe it would be a good test case to include in continuous testing? big_problem_file.txt

Interestingly, the latter file again has the issue reported in issue #144 (<= at position 1023 in a line). So I guess the latest input parsing change re-exposed some issues at the buffer length of 1024, again?

In any case, thanks for looking into all these oddities!

jjhforrest commented 3 years ago

David,

I am unable to reproduce errors. Can you do a completely fresh build - just to make sure. What operating system and compilers are you using (I use Linux Ubuntu)?

John Forrest On 19/02/2021 17:57, David Laehnemann wrote:

After the latest input parsing update https://github.com/coin-or/CoinUtils/commit/86ab8d39f90f3119ce372ff88c9e62b54dc2cfe9, my larger problem file now turns up this problem:

|Welcome to the CBC MILP Solver Version: 2.10.5 Build Date: Feb 19 2021 command line - /path/to/bin/cbc factor_starting_at_1025.lp solve (default strategy 1) CoinLpIO::readLp(): Maximization problem reformulated as minimization ### CoinLpIO::is_invalid_name(): Name 1183 should not start with a number ### CoinLpIO::are_invalid_names(): Invalid name: vnames[59]: 1183 ### CoinLpIO::readLp(): Invalid column names Now using default column names. |

A minimal example to reproduce this is: factor_starting_at_1025.txt https://github.com/coin-or/CoinUtils/files/6011911/factor_starting_at_1025.txt

But for more extensive input testing, I'll just include the larger input file that I have been extracting all these cases from. The model itself doesn't actually make much sense in that form, but the format should be alright. So maybe it would be a good test case to include in continuous testing? big_problem_file.txt https://github.com/coin-or/CoinUtils/files/6011948/big_problem_file.txt

Interestingly, the latter file again has the issue reported in issue

144 https://github.com/coin-or/CoinUtils/issues/144 (|<=| at position

|1023| in a line). So I guess the latest input parsing change re-exposed some issues at the buffer length of 1024, again?

In any case, thanks for looking into all these oddities!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/coin-or/CoinUtils/issues/147, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJYHAVENZLE46BINBEJETS72RBBANCNFSM4X4ZBFZQ.

dlaehnemann commented 3 years ago

Thanks for checking.

I am also on a Ubuntu machine (18.04 LTS) and I thought this was what I did. But let me do this again. Deleted all the old fetched repos and the build directories and then follow build instructions with coinbrew.

So I double-checked if I have all the dependencies mentioned there and it seems I had been lacking BLAS. So that is now installed. Then I did:

git clone https://www.github.com/coin-or/coinbrew
cd coinbrew
./coinbrew build Cbc@master --prefix=/path/to --enable-debug --no-prompt

This builds and installs to /path/to without complaint. Then running this (or the big_problem_file.lp):

/path/to/bin/cbc factor_starting_at_1025.lp solve

I actually now get a SIGABRT:

Welcome to the CBC MILP Solver
Version: devel
Build Date: Feb 19 2021

command line - /path/to/bin/cbc factor_starting_at_1025.lp solve (default strategy 1)

ERROR while running Cbc. Signal SIGSEGV caught. Getting stack trace.
/path/to/lib/libCbcSolver.so.0(_Z15CbcCrashHandleri+0x154) [0x7f1811ad86a4]
/lib/x86_64-linux-gnu/libc.so.6(+0x3efd0) [0x7f18112eafd0]
/path/to/lib/libCoinUtils.so.0(+0x65610) [0x7f1810bce610]
[0x7ffce299a080]

ERROR while running Cbc. Signal SIGABRT caught. Getting stack trace.
/path/to/lib/libCbcSolver.so.0(_Z15CbcCrashHandleri+0x154) [0x7f1811ad86a4]
/lib/x86_64-linux-gnu/libc.so.6(+0x3efd0) [0x7f18112eafd0]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7) [0x7f18112eaf47]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x141) [0x7f18112ec8b1]
/path/to/lib/libCbcSolver.so.0(+0x3dba7) [0x7f1811a80ba7]
/lib/x86_64-linux-gnu/libc.so.6(+0x3efd0) [0x7f18112eafd0]
/path/to/lib/libCoinUtils.so.0(+0x65610) [0x7f1810bce610]
[0x7ffce299a080]

Aborted (core dumped)

And even though I built with --enable-debug, the dumped core file doesn't seem to have debug symbols, so I can't really tell why.

Now retrying without the --enable-debug...

dlaehnemann commented 3 years ago

Still the same without --enable-debug.

Not sure what happened, here. But I guess that it works on your end is a good sign. Will have another look at the beginning of next week.

jjhforrest commented 3 years ago

I have reworked code to get rid of ungetc hack. It is possible that is buggy sometimes.

The memory defined in CoinLpIO.hpp has changed, but coinbrew will not automatically re-compile all .cpp files including this file if they are not in CoinUtils - so you will need to do afresh build. Not doing that I get the exciting error message -

stack smashing detected : terminated

John Forrest

On 20/02/2021 00:06, David Laehnemann wrote:

Still the same without |--enable-debug|.

Not sure what happened, here. But I guess that it works on your end is a good sign. Will have another look at the beginning of next week.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/coin-or/CoinUtils/issues/147#issuecomment-782466879, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWJYHGJQCPUDXT2OXLYBVLS734JLANCNFSM4X4ZBFZQ.

dlaehnemann commented 3 years ago

That sounds very dramatic, indeed... :D

I had been re-downloading and re-building everything from scratch all the time, to eliminate any possiblity of picking up old stuff. But your latest changes seem to have done the trick. On the machine I was originally testing (and also on another machine that I have access to today), compiling the latest Cbc@master with coinbrew gives me a cbc executable that gets through reading and solving all of my problem cases.

Thanks again for keeping at this! And let us hope, no more stacks will be smashed in the making of this software...

tkralphs commented 3 years ago

Just to be clear, coinbrew is nothing more than a wrapper that makes it easy to fetch the source and build/install the projects in the right order using the underlying autotools build harness. Underneath, it is just calling make, which should re-build everything that needs to be re-built. However, I am invoking configure with the option --disable-dependency-tracking for some reason that is lost to history and this could mean that if a header is changed, then not all source files depending on that header get recompiled. You could try to remove all occurences of the string --disable-dependency-tracking from coinbrew and if everything works better, maybe I can just remove it.

On the other hand, coinbrew does also have a "rebuild" option, which cleans a project first, so you can do coinbrew build Cbc --rebuild --skip-dependencies and it should clean only that project and then rebuild it.

If all else fails, you can also always drop down into the build directory for a given project and make clean; make to just re-build that single project.

dlaehnemann commented 3 years ago

Thanks for the insight. And coinbrew is a very handy wrapper at that! I really like the config.yml setup for the dependency specification.

One thing that initially confused me a bit, was that the build instructions with coinbrew (at https://coin-or.github.io/user_introduction) are not linked to anywhere on https://www.coin-or.org/ that I cound find...

And I know the "re-download and re-build everything" strategy is a bit of a sledge hammer. But even with all the dependencies, it goes reasonably fast. So that was my quickest way of dealing with this, instead of looking into coinbrew options. And I guess in this case, where I would need the newest Cbc AND the newest CoinUtils, I would have to go without the --skip-dependencies, anyway, right?

tkralphs commented 3 years ago

There is a link to https://coin-or.github.io/user_introduction on line 71 of the README, though looking at it now with fresh eyes, it doesn't stand out very much, so maybe I will do a small update to the README to make it more obvious and also include it down in the list of links to documentation at the bottom.

Yes, if you want to rebuild the whole stack for a new version, you would leave off --skip-dependencies. You could do something like

coinbrew fetch Cbc@master
coinbrew build --rebuild Cbc

but that incantation might not go that much faster than just starting from scratch. You would save the configuration time, though, which is the only part of the build that can't be parallelized (if you're not doing parallel builds, that speeds everything up substantially).

Thanks for affirmation on config.yml. That's my proposed replacement for what are currently two different configuration in ancient formats: the Dependencies file (which is in the format it's in because that is the format subversion uses for specifying the svn:externals property) and the projDesc.xml file, which was used to auto-generate project descriptions for the main Web site. Although the README strongly implies that config.yml is already being parsed by coinbrew, I haven't actually rolled that feature out yet. There is a version of coinbrew in the yaml branch that implements it and I would be really happy to have some testing done on it. The lack of much testing (and bandwidth) is one of the reasons it has not been rolled out fully yet (also, there are other COIN-OR projects that don't yet have a config.yml file). Let me know if you're interested in being a beta tester.

dlaehnemann commented 3 years ago

Yeah, that link in the READMEs eventually got me there, so that was already quite helpful. I guess I shouldn't have spent too much time poking around the coin-or.org website, looking for general docs there.

I have now come across yaml configuration files in a number of contexts, and find them a very good combination of human readable and machine readable. Among the next builds I'll surely be doing for my Cbc-dependent project, I'll try a build from coinbrew's yaml branch to see how that works and report in coinbrew repo, if I come across anything.

Thanks again for all the insights -- and all the volunteer work on this project!

tkralphs commented 3 years ago

Ah, yeah, I see what you were getting at about the documentation. Yeah, the COIN-OR Foundation is a separate beast and at least in principle, it is completely decoupled from the management of individual projects. The documentation you're talking about is only for projects that are part of the "Optimization Suite," which is just a subset of the projects hosted by COIN-OR. It often gets convoluted with the foundation itself, since there is a big overlap between the people involved in the foundation and the people involved in development of the Optimization Suite. But the open source projects that are maintained under the auspices of the COIN-OR Foundation go beyond just those in the Optimization Suite and it is not really the Foundation's role to get involved in management of individual projects. Hence, the lack of pointer to specific documentation.