-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
highs-js internal exception with >= 32 variables ? #3
Comments
Hello ! I comment on myself here in case someone else has the same issue: the LP problem must be provided in CPLEX LP format, which comes with some nasty restrictions on the length of the lines => http://web.mit.edu/lpsolve/doc/CPLEX-format.htm#:~:text=The%20CPLEX%20LP%20file%20format,format%20that%20lpsolve%20can%20read. So, this is working instead (notice the new line in the objective function definition):
Still, I think this issue is still valid in order to improve the output to the user in case of error. Here, I suspected a problem of length from the behaviour, but for others potential issues (memory errors, model file writting or reading to/from disk impossible, ...), you might want to provide the full output from the C program to Node.js. Cheers, Roman |
@lovasoa Is the LP file is being read by highs-js, or passed to HiGHS itself? The HiGHS LP file reader only handles the default maximum line length - as exposed by another user a few months ago. Whatever, I'm prompted to see what error return is given by HiGHS if the LP file line length is exceeded. |
The LP file is parsed by highs, not by this library. I'm closing this since it's a bug in HiGHS. |
The error reporting issue is also a bug in HiGHS. HiGHS aborts instead of returning a clean error when calling |
What's the githash of the version of HiGHS that you're using. I'm looking at what happens in highs/master, but it doesn't abort. This suggests that the fix that was put in a few months ago isn't in the version you're using. More generally, we're working towards a first formal release of HiGHS, so we'll have proper version numbers to refer to! |
HiGHS is a git submodule of this repo. You can see the exact version used, and open a pr to update it. |
@jajhall : Since you are a maintainer of HiGHS, can I add you as a maintainer on this project too ? |
Oh, I see you already are ^^ |
You you share your test ? |
I think that this is why I got the initial notification! I've looked through the behaviour of Highs/master, and Highs_readModel should return a value 2 (Error). Are you interpreting this? Just one thing makes me cautious. I'm testing on Linux, and the LP file reader handles the excessive line length by throwing an exception that is then caught, allowing HiGHS to return the error. This seems to be a standard coding approach, so I'd expect it to be OK on Windows/MacOS |
I've created a .lp file from the text above and am just running the HiGHS executable. |
Thanks. New ground for me on GitHub, but I see that you appear to be using HiGHS @ 91e72a0. Specifically A useful exercise in several ways! |
highs-js uses the C interface, not the HiGHS binary directly.
This repo uses https://github.com/ERGO-Code/HiGHS/tree/91e72a00ef215fbee49af10efe213087490efa1e from March 18. |
So, given my comment made just before your last one, there's little more that HiGHS can do in this case - other than setting the model_status to correspond to a load error - rather than leaving it "unset". The line limit for .lp files is very unfortunate, particularly as I think I'm right in saying that CPLEX itself doesn't apply the standard, and reads files with any line length! Of course, for toy LP problems, it's not an issue - and then folk do what your used has done and solvers stop working! |
Is there a reason to keep this limit in highs ? |
HiGHS could return a meaningful error code and add a function to the C api associating error code to error messages. |
The limit is part of the file format standard, so we'll keep it for now. We've not had much interaction with .lp file users so it hasn't been much if a priority As for error return codes, a more sophisticated system would be in order - I don't know what Gurobi offers - but, again, it's not a priority. There is, of course, an error code returned that you don't seem to have interpreted 🙂 |
The current code does raise a proper js error when readModel does not return 0. The problem is on HiGHS side, it doesn't return a proper error and aborts instead. |
I improved error handling further in #4, but it will still require Highs_readModel to return a proper error instead of crashing. |
Ah, I see now! HiGHS returns an error with my Linux and Windows builds, but crashes on your architecture. I realise that it's embedded within js, but how (and on what architecture) is HiGHS compiled? I can create a branch to introduce print statements so that you can help me to identify where HiGHS crashes. Is this feasible? That said, my instinct is that it's something to do with HiGHS throwing an exception that is then caught. Eliminating this standard method of error-handling is non-trivial. |
Yes, wasm doesn't support exceptions. Emscripten has an option to add a js wrapper around each exception, but with a high performance impact: wasm isn't the only target that doesn't support exceptions, especially in the embedded world. |
To prevent any exceptions raised in HiGHS from completely crashing the browser/node.js process, would there be any possibility to catch the wasm errors (RuntimeError: abort(5406152)) inside the .solve() function ? |
Thanks. I see that you've been doing things to highs-js to work around this. |
@lequant40 Yes, I added that and published a new version. |
@jajhall Thanks ! |
You're welcome @lovasoa . This has exposed the need for us to document the (limited) use of exceptions in HiGHS, and to ensure that all internal calls to the IPM solver are protected by "try {}" and exceptions are handled properly. This is how we all win in the open source world! |
Hello !
Trying to play with the package, it seems there is an error when I try to increase a very simple problem from 31 variables (working OK) to 32 variables (exception).
Would it be possible you have a look ?
My reproducer is:
31 variables => OK
32 variables => KO
Seems an exception is being issued by either the underlying Node.js glue to the C program, but with no more information.
Cheers,
Roman
The text was updated successfully, but these errors were encountered: