On Wednesday, May 27, 2015 at 1:02:06 AM UTC+5:30, Waldek Hebisch wrote:
>
> > 
> > Sorry for the silence. So, the GSoC coding period has started and I've 
> > finished the implementation of the newtonpolygon 
> > function [1]. Could you please review it? 
>
> Some remarks: 
>
> 1) It make sense to pass argument of type LODO3 to newtonpolygon. 
>    In fact, it makes sense to pass operator having Laurent series 
>    coefficient.  So convertion from LODO1 and change of coefficient 
>    would be at upper level 
>
 
Yes, in fact I observed it while working on the factor_newton function 
which implements the comprime index 1 factorizations
algorithm where the input is a LODO3 and newtonpolygon is used. I've 
changed it now. 

2) There is a two argument version of order function: 
>
>      order(c, n) 
>
>    will give you min of order(c) and n.  So no need to test if 
>    c is 0 (in particular this avoids problems when c has infinite 
>    number of zero coefficients). 
>
 
Okay, thanks.
 

> 3) You replace missing coefficients of the operator by 0.  This 
>    is OK if leading coefficient of the operator is has order 0 
>    but may be wrong if operator has positive order.  Using order 
>    of leading coefficient instead of 0 would be OK, but probably 
>    it is better just work only with coefficients that are present. 
>
 
Oh, I wrote it assuming f to be monic which some of the algorithms that
use newtonpolygon have as a pre-condition. However, this is redundant now
that I only use the coefficients actually present.

4) You use a lot indexing on lists.  In general this is inefficient 
>    and frequently means that array would be more appropriate. 
>    In this case intead of indexing use interation over lists.  
>

I use iteration now.
 

> 5) It makes sense to split newtonpolygon into smaller functions. 
>    In particular extracting coordinates of points, computing convex 
>    envelope and computing polys associated to slopes could be done 
>    by separate functions. 
>

Done.
 

> 6) I do not see why you have "failed" as part of the result? 
>    Whithout it the result would be simpler. 
>

Yes, but what should I do about the final point in the polygon? It doesn't 
have any
slope or Newton polynomial associated with it. Should I just set them to 0?

 

> 7) AFAICS you use quadratic method for computing convex envelope. 
>    There is simple linear method.  Namely given points a1, ..., an 
>    given in increasing order of x-coordinate we proceed as follows. 
>    We keep a tentative list of points with slopes [[p1, s1], ..., [pk, 
> sk]]. 
>    At the beginning we accept [a1, 0] to tentative list.  Then we 
>    handle a2, ..., an iteratively.  In itertaion dealing with al we 
>    compare slope between pk and al and sk.  If sk is bigger or equal, 
>    then we drop [pk, sk] from tentative lists.  We keep dropping points 
>    and stop when we find [pi, si] with slope between pi and al bigger 
>    than si.  Then we add al with the slope between pi and al to 
>    tentative list. 
>
>
I've tried to change the function according to the above. [1]
 

> > Here are some known issues so far: 
> > 
> > 1. lodo3.spad and task2.spad are unnecessary as the contents of 
> > lodo3.spad can be pushed into lodo.spad; however, 
>
> I will add lodo3.spad to lodo.spad 
>
>
Okay.
 

> > I'm not so sure about 
> > contents of task2.spad. Any ideas on where the package should go and 
> > what it's name should be? 
>
> It is related to lodo3.spad, so it makes sense to put it in the 
> same file.  Concerning names, I will probably call it 
> 'LODOConvertions' and call the exported operation just 'convert'. 
>
>
Okay, thanks.
 

> > 2. On compiling the function, I get the following: 
> > Warnings: 
> >        [1] newtonpolygon:  y has no value 
> >        [2] newtonpolygon:  x has no value 
> > Though this doesn't seem to cause issues related to the correctness of 
> > the answers, it'd still be good if I could make it go away. 
>
> This is supposed to mean that z and y are used uninitilized.  But 
> in your case warning seems spurious. 
>
>
Well, the warnings seem to keep increasing. Here are the new ones:
[1] newtonpolygonPoints: not known that (LeftModule UP) is of mode 
(CATEGORY domain (SIGNATURE coerce ($ (Variable var))) (SIGNATURE 
differentiate ($ $ (Variable var))) (IF (has F (Algebra (Fraction 
(Integer)))) (SIGNATURE integrate ($ $ (Variable var))) noBranch))
[2] newtonpolygonPoints:  points has no value
 

> -- 
>                               Waldek Hebisch 
> [email protected] <javascript:> 
>

[1] 
https://github.com/fandango-/fricas/commit/e58c35b7d96db28746b7effb6c07da2977288abc

Thanks,
Abhinav. 

-- 
You received this message because you are subscribed to the Google Groups 
"FriCAS - computer algebra system" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/fricas-devel.
For more options, visit https://groups.google.com/d/optout.

Reply via email to