-
Notifications
You must be signed in to change notification settings - Fork 199
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
master (v.1.1.0) vs develop (3/12/2020) #479
Comments
What error do you get? Please be aware that the current version in
15% is quite significant! |
See comments below.
Op 13 mrt. 2020, om 18:36 heeft Christoph Zurnieden <notifications@github.com<mailto:notifications@github.com>> het volgende geschreven:
[…]the error on mp_set_double compilation that master (v.1.1.0) branch was giving[…]
What error do you get?
I guess it’s the MacOS issue.
There is a problem with MacOS, see #159<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_libtom_libtommath_issues_159&d=DwMFaQ&c=XYzUhXBD2cD-CornpT4QE19xOJBbRy-TBPLK0X9U2o8&r=5ZdsQceb57-oiBH6dKANmB5EQ29Bw79DlNhWrSmRRjI&m=omJsp-ldy4OV2mrliQj2joS3O44BCBbVKuibFpzSCWs&s=3_NMkdM9Tov0fb3qrIXAobpsFJQUpQFBnDXRi_q9vsk&e=> for the details. The actual commit to fix it is in #476<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_libtom_libtommath_pull_476&d=DwMFaQ&c=XYzUhXBD2cD-CornpT4QE19xOJBbRy-TBPLK0X9U2o8&r=5ZdsQceb57-oiBH6dKANmB5EQ29Bw79DlNhWrSmRRjI&m=omJsp-ldy4OV2mrliQj2joS3O44BCBbVKuibFpzSCWs&s=r4NRl2shp6-WzRFsptEzOUcFewnYb5mUSZxgGZ81Bms&e=>
Ok, I’ll have a look.
Please be aware that the current version in master is 1.2.0. I don't know if the fix from #476<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_libtom_libtommath_pull_476&d=DwMFaQ&c=XYzUhXBD2cD-CornpT4QE19xOJBbRy-TBPLK0X9U2o8&r=5ZdsQceb57-oiBH6dKANmB5EQ29Bw79DlNhWrSmRRjI&m=omJsp-ldy4OV2mrliQj2joS3O44BCBbVKuibFpzSCWs&s=r4NRl2shp6-WzRFsptEzOUcFewnYb5mUSZxgGZ81Bms&e=> gets backported in the near future, we are currently a bit understaffed and overworked here, so it might take a little more time to get 1.2.1 out.
Will check against 1.2.0 as well.
Furthermore, when I use the develop version my factorial algorithm is say 15% slower with develop than with master. Any idea why?
15% is quite significant
What algorithm? Naive, binary splitting, Borwein, Borwein-Schönberg?
Nothing fancy, ordinary multiplication, but I’ll investigate further.
See snippet below (Mbiginteger is defined as mp_int)
Mbiginteger* _result=_getBiginteger(6); // the smallest value to return
if(_result){
// we could store fac values in a special list with index equal to the argument, in which case we could look up the starting value
// we could start at some intermediate value????
Mbiginteger *_multiplier=_getBiginteger(3);
if(_multiplier){
while(mp_cmp(_multiplier,_finalmultiplier)==MP_LT){
if(mp_incr(_multiplier)!=MP_OKAY){outputError("Failed to increment a big integer");_result=NULL;break;} // if we fail to increment break
if(mp_mul(_result,_multiplier,_result)!=MP_OKAY){outputError("Failed to multiply a big integer");_result=NULL;break;}
//////////if(amVerbose())outputBigInteger("Result so far: '",result,"'.");
}
// get rid of intermediate big integers
free_biginteger(_multiplier);
}else
outputError("Failed to create big integer 3");
}else
outputError("Failed to create big integer 6");
free_biginteger(_finalmultiplier);
if(amVerbose())outputBiginteger("Result of applying the factorial() function: '",_result,"'.\n");
return (_result?_getBigintegerValue(_result,true):NULL);
Do you have the implementation public?
No, it’s still private.
(I apologize in advance if you have it in your github repository and I was too blind to find it)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_libtom_libtommath_issues_479-23issuecomment-2D598836331&d=DwMFaQ&c=XYzUhXBD2cD-CornpT4QE19xOJBbRy-TBPLK0X9U2o8&r=5ZdsQceb57-oiBH6dKANmB5EQ29Bw79DlNhWrSmRRjI&m=omJsp-ldy4OV2mrliQj2joS3O44BCBbVKuibFpzSCWs&s=D2_S-msH9vmTO_22LZXJ59w448CQfXWIo8tvV0LKVM4&e=>, or unsubscribe<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ACELO24SOJFVGXUSQLU3QYLRHJVIPANCNFSM4LHCLBNQ&d=DwMFaQ&c=XYzUhXBD2cD-CornpT4QE19xOJBbRy-TBPLK0X9U2o8&r=5ZdsQceb57-oiBH6dKANmB5EQ29Bw79DlNhWrSmRRjI&m=omJsp-ldy4OV2mrliQj2joS3O44BCBbVKuibFpzSCWs&s=Mgs3rcqzJuviYOT_ntUD3SxG-hTETgCW2LKsANLtV7Y&e=>.
|
I'm closing this as there was no action for too long. |
I'm using libtommath to some extent in an interpreter I'm writing, and because the current develop branch was not giving me the error on mp_set_double compilation that master (v.1.1.0) branch was giving me, I was intent on switching over to using develop instead. However, some of the functions I was using have turned into defines (mp_incr, mp_decr, mp_isodd, mp_sqr); mp_n_root was renamed (I guess) to mp_root_n and mp_toradix to mp_to_radix; MP_YES and MP_NO were removed. Using the develop tommath.h was much worse than an adaptation of the old tommath.h, so I switched to using the latter. One problem I encountered was that the type of the size parameter in mp_radix_size changed from int to size_t, causing serious problems when passing in a local int (overwriting a pointer). For now I've added a compiler flag to check whether to use size_t or int but it is a nuisance to have to put that in a C source file:
#ifdef M_MP_DEVELOP
size_t arepsize=0; // MDH@13MAR2020: changing type int to size_t (which is larger), so that we should be able to use it with libtommath-develop
#else
int arepsize=0;
#endif
Furthermore, when I use the develop version my factorial algorithm is say 15% slower with develop than with master. Any idea why?
Kind regards,
Marc P. de Hoogh
The Netherlands
The text was updated successfully, but these errors were encountered: