[PATCH] cleanup "*" in MRING

[oldk1331]

The local function "sortAndAdd" is almost the same as
"construct", but less efficient: it creates new "Term"
instead of using existing ones.

Also, the usage of "+" in "*" can also be omitted:
we can use the newly added "concat!" and pass the whole
list to "construct!".

[Waldek Hebisch]

It is not clear for me that we can do this. Existing code
is quite careful to eliminate terms with zero coefficient.
In your code it is not clear if/how this is done.

--
Waldek Hebisch

[oldk1331]

Isn't "construct" carefully eliminate terms with zero coefficient?

--
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 fricas-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fricas-devel/E1hqHkK-0000TH-Oe%40hera.math.uni.wroc.pl.

[Waldek Hebisch]

oldk1331 wrote:
>
> Isn't "construct" carefully eliminate terms with zero coefficient?

'construct' for MonoidRing is should eliminate terms with zero
coefficients, but IIUC it is quite slow. You called 'construct!'
which seem to be defiend only for aggregates. For aggregates
eliminating zero coefficients would be incorrect...

--
Waldek Hebisch

[oldk1331]

"construct" is not slow, it just sorts its argument

and scans it once. "construct!" just skips one

copy. They are faster than "sortAndAdd" or "+".

--
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 fricas-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fricas-devel/E1hqJFL-0001SC-NR%40hera.math.uni.wroc.pl.

[oldk1331]

Actually, it's not necessary to treat differently for
OrderedMonoid/noZeroDivisors.  We can simply have

    a : % * b : % ==
        zero? a or zero? b => 0
        construct! concat! [[[ta.Mn*tb.Mn, ta.Cf*tb.Cf]$Term
            for tb in b] for ta in a]

we collect all the terms and let "construct" do the heavy lifting.
See my updated patch.

It's possible to do better though, for OrderedMonoid, we can do a
k-way merge, but we don't have such algorithm at hand.

My patch should already make this function much faster by
skipping allocation of temporary result of "+".

[oldk1331]

On 7/25/19 7:51 PM, oldk1331 wrote:
> Actually, it's not necessary to treat differently for
> OrderedMonoid/noZeroDivisors.  We can simply have
>
>     a : % * b : % ==
>         zero? a or zero? b => 0
>         construct! concat! [[[ta.Mn*tb.Mn, ta.Cf*tb.Cf]$Term
>             for tb in b] for ta in a]
>
> we collect all the terms and let "construct" do the heavy lifting.
> See my updated patch.

There is a serious problem in "construct" though, it may
modify the content of a "Term":

t1.c:= t1.c+t2.c

We should create new cells instead of reusing old ones.