On 09/10/2015 07:32 AM, Abhinav Baid wrote:
>> PS: I'll try to produce a patch to change Set to List in lodof2.spad. 
>>
> 
> I just did that. [1]
> There was a Set FA in the code too, so I changed that also.
> 
> [1] 
> https://github.com/fandango-/fricas/commit/81ca9a8067d94c1287dd98f4dec3df02b8e199ac
>  
> 
> 
> Thanks,
> Abhinav.

Thank you. And yes, now the Aldor stuff compiles, but I wonder why it
already works.

Aldor should probably have complained about this statement as well.

https://github.com/fricas/fricas/blob/master/src/algebra/lodof2.spad#L281

    semi : Table(FA, UP) := table()

since we have

  FA ==> Record(factor : UP, exponent : Integer)

and

https://github.com/fricas/fricas/blob/master/src/algebra/table.spad#L99

Table(Key : SetCategory, Entry : Type) : Exports == Implementation where



Anyway, Abhinav, I'd like to make some comments to your code.

https://github.com/fricas/fricas/blob/master/src/algebra/lodof2.spad#L284

                for i in v for ii in 1..(#v)-1 repeat
                    restl := rest(v, ii)

This looks somewhat ugly to me. Since v is a list of factors, one can
assert that it's never the empty list, so something like

  i := first v
  while not empty?(v := rest v) repeat
      -- Your code here with 'restl' replaced by just 'v'.
      -- There shouldn't be '=> "iterate"' then, because it would
      -- miss the following statement.
      -- Since in other places of this loop you also do not use '=>'
      -- to restart iteration, for me it feels like some change in
      -- coding style that makes the code harder to read.
      i := first v

'rest(v, ii)' unnecessarily introduces linear time.

As for the variable 'semi'. It is only important if 'option="semireg".
Feels to me like you are wasting time computing 'semi' it if this option
is not set.

Furthermore, in case of "semireg", you use 'semi' as a Table to have
direct access to semi.i or semi.j. But do your really need it? The
current implementation of Table falls back to AssociationList for your
Key type and that has linear time access. Maybe better avoid Table.

In the end you remove all the unsafe factors from v. I've now not looked
at the paper, but it looks a bit suspicious that your code can put i
several times into the 'unsafe' list. That means at the same time that
the factor i is added to semi(j) for two different j's.

Furthermore, it can happen that one j puts i into the 'unsafe' list and
the same i (although now "unsafe") puts a later j  to the 'unsafe' list.
Is that behaviour OK?

Ralf

-- 
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