On Apr 2, 2011 10:58pm, Jordi Gutiérrez Hermoso <address@hidden> wrote:
> This is a bit more than a patch. It's a considerable rewrite of the > GLPK code. I am certainly not able to evaluate it myself for > correctness. I can only comment on superficial things. However, I > wouldn't want to dismiss this effort either.
Hi Jordi, thanks for your prompt reply. Let me provide you with a bit of background information about what I did with my patch. There is a tool that Nicolò maintained more actively throughout the years and it's GLPKMEX, an interface to GLPK for MATLAB. For reasons that I personally ignore, Nicolò is currently not maintaining the code himself any longer. The project is still alive though and is being maintained by Niels Klitgord. I personally used such tool and I indeed missed the possibility to address binary problems from Octave too. So what I did was to take GLPKMEX, to fix what as far as me could have been improved and then port it to Octave. As long as GLPKMEX goes, I think Nicolò's approval is there. As for my changes, I contacted Niels about them. He's kind of busy with something else at the moment, so he couldn't go through the patch yet, but I'm confident he'll have a look eventually. In any case, they aren't very extensive.
> There are a few cosmetic things about your rewrite, like indentation > rules. Please follow GNU style guidelines. Also, your patch seems to > remove a lot of conditional compilation checks based if the system has > GLPK or not. Was that intentional?
The code I started working on was poorly aligned, I just passed the thing through astyle, ansi style. But I can certainly make it look more "gnuish". As for the precompiler issue, the code relies on capabilities present in GLPK 4.36+. Checking for GLPK 4.14 wouldn't make much sense; furthermore for 4.36+ the message hook is only one. And this explains why some conditions are removed. As for whether I don't disable the function from within the code with other conditionals if the right version isn't there, for how I see it the best thing to do would be to check at configure time whether there's the right version of GLPK installed. This would lead to disable GLPK alltogether if GLPK < 4.36. It is a bit drastic, but GLPK changed internally. So a distinct "either GLPK 4.36+ or nothing" isn't too bad of a compromise. Let's also remember that GLPK 4.36 is already 2 years old, so it's not exactly new.
Concerning how to check the GLPK version at configure time, I didn't venture into changing configure.ac myself, but the test to check this is straightforward
// BEGIN TEST
#include <glpk.h>
int main() { int result = 0;
if (GLP_MAJOR_VERSION < 4) { result = 1; } else { if(GLP_MINOR_VERSION < 36) { result = 1; } }
return result; }
// END_TEST
and can be easily performed with AC_RUN_IFELSE. If somebody could eventually add this, I'd appreciate it.
> Also, it would be nice if you could provide an hg changeset following > the suggestions in my blog post. Please let me know if the > instructions are unclear: