Hi Heinrich, thanks a lot for the code review.
> so your current changeset is in branch get_exact_values. > https://github.com/nicodelpiano/glpk/commits/get_exact_values Yes, I will be working there for this enhancement. > Do we need three different scmp structures and three initalization routines? No, of course. I just did it that way as I was trying to not break anything from the previous functionality. Now I have integrated them into the `glp_smcp` structure. > You should move the check to before malloc. Sure, I don't know why I did it before the NULL check. > Why do you malloc at all? You are calling mpz_get_str with str = NULL. You are right. I have changed that and works fine for the exact rational results. For the fixed precision case, if I do not malloc here https://github.com/nicodelpiano/glpk/commit/693d02b81fc2b7608cf1087cd31bbeb56f6390ea#diff-152a5a13f47885a5d7b50b4ed1db48a8R309 then the statement https://github.com/nicodelpiano/glpk/commit/693d02b81fc2b7608cf1087cd31bbeb56f6390ea#diff-152a5a13f47885a5d7b50b4ed1db48a8R470 will not work. > Can't the result transfer be integrated in the existing glp_exact() function? Yes, definitely, I have put them into that function. > mpz_get_str should be called with a custom allocator using xmalloc and > xfree. Use mp_set_memory_functions to set custome allocation function. I tried to do this, but I couldn't make it work. I guess this customization should be done before any GMP allocation (when there are no active GMP objects) and that is why I am getting errors. Is it too bad to leave the default allocation instead? If you have any suggestions please let me know. > smcp->exact_size is only used in malloc statements. That is strange. > Shouldn't the following code consider the value you have chosen for > exact_size? I have changed the things a bit. I put three parameters in `glp_smcp` structure that I think are the ones we need: - num_states: this specifies the number of states of the solutions - precision: an integer that specifies the precision in bits - n_digits: number of decimal digits (this is used here https://github.com/nicodelpiano/glpk/commit/693d02b81fc2b7608cf1087cd31bbeb56f6390ea#diff-152a5a13f47885a5d7b50b4ed1db48a8R470 we can get rid of this parameter) Another thing I changed are the calls to `set_d_eps`: as I needed the exact results I put `mpq_set_d` when loading data, for example here https://github.com/nicodelpiano/glpk/commit/693d02b81fc2b7608cf1087cd31bbeb56f6390ea#diff-152a5a13f47885a5d7b50b4ed1db48a8R165 If GMP is not active we should put this between HAVE_GMP flag, am I right? Sorry for the excessive amount of links, if something is not clear enough, let me know. Thanks! Nico. 2016-07-13 17:19 GMT-03:00 Heinrich Schuchardt <[email protected]>: > Hello Nico, > > so your current changeset is in branch get_exact_values. > https://github.com/nicodelpiano/glpk/commits/get_exact_values > > Thank you for your contribution. > > I have started reviewing the changes. Some parts of the code may have to > be adjusted before Andrew can integrate them: > > Do we need three different scmp structures and three initalization > routines? I suggest to add the additional parameters to the existing > smcp structure. > > smcp->exact_size is only used in malloc statements. That is strange. > Shouldn't the following code consider the value you have chosen for > exact_size? > > You dereference parm for malloc. Afterwards you check if it is NULL. > You should move the check to before malloc. > > Why do you malloc at all? You are calling mpz_get_str with str = NULL. > So you request allocation of memory here. > > smcp is a parameter structure. You should not put results there. > Why can't you use the glp_prob structure for transfering the results? > > Do we really need glp_exact_sol()? Can't the result transfer be > integrated in the existing glp_exact() function? > > mpz_get_str should be called with a custom allocator using xmalloc and > xfree. Use mp_set_memory_functions to set custome allocation function. > This will make it easier to identify mermory leaks. > > When I checked out the branch it contained a superfluous file "prec". > > Best regards > > Heinrich Schuchardt > > > On 07/12/2016 03:39 PM, Nico Del Piano wrote: >> Hello Heinrich, >> >> thanks for your fast reply and sorry for the code, yes, it was a complete >> mess! >> >> I created a new cleaned branch, this is the specific commit >> https://github.com/nicodelpiano/glpk/commit/da11fac3996d9ed1012f1c50e76971e20f2b343e >> >> I am using gmp, so I first configure the project with --with-gmp. >> >> Best regards, >> >> Nico. >> >> 2016-07-12 9:48 GMT-03:00 Heinrich Schuchardt <[email protected]>: >>> Hello Nico, >>> >>> your mail had to be forwarded manually because you're not subscribed to the >>> GLPK Help list. >>> >>> In GitHub the history of the exactsolver branch is messed up. >>> >>> You have been committing all sorts of files which are not source like >>> configure.log, •.o, ... >>> >>> I cannot find any meaningful commit messages. >>> >>> Please, create a clean branch. >>> >>> Best regards >>> >>> Heinrich Schuchardt >>> >>> http://www.xypron.de >>> >>> Am 12.07.16 um 14:35 schrieb Andrew Makhorin >>> >>>> -------- Forwarded Message -------- >>>> From: Nico Del Piano <[email protected]> >>>> To: [email protected] >>>> Subject: Interface for exact solutions >>>> Date: Tue, 12 Jul 2016 08:44:39 -0300 >>>> >>>> Hi all, >>>> >>>> I'm using the exact simplex solver provided by glpk to compute exact >>>> solutions from a project written in Java (using the Java bindings >>>> java-libglpk) and I haven't found a way to retrieve those exact >>>> values: I'm getting double precision solutions. >>>> >>>> To overcome this, I defined a not-so-pretty interface to store these >>>> exact values (I provided two ways to do this, both for rational >>>> precision and fixed precision) here [1]. Basically, I just copied the >>>> `glp_smcp` structure and added some parameters to properly store the >>>> solutions in the desired precision. I also added two solvers >>>> `glp_exact_sol` (for rationals) and `glp_fixed_sol` (for user-defined >>>> precision). For example, if we use the `glp_fixed_sol` solver, we can >>>> specify in the structure the precision we want, and store there the >>>> solutions, in order to get those values later on. >>>> >>>> Obviously, this doesn't seem to be the best approach, so the purpose >>>> of this email is to discuss how to improve this interface, or, even >>>> better, how to add this feature in glpk. Furthermore, if you think >>>> this can be done in glpk and I'm just ignoring it, please let me know. >>>> >>>> Thanks, >>>> >>>> Nico. >>>> >>>> [1]: https://github.com/nicodelpiano/glpk/tree/fixed_precision >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> Help-glpk mailing list >>>> [email protected] >>>> https://lists.gnu.org/mailman/listinfo/help-glpk >> _______________________________________________ Help-glpk mailing list [email protected] https://lists.gnu.org/mailman/listinfo/help-glpk
