Project

General

Profile

Design #686

myAddMul in poly rings

Added by John Abbott about 9 years ago. Updated about 9 years ago.

Status:
New
Priority:
Normal
Assignee:
Anna Maria Bigatti
Category:
Tidying
Target version:
CoCoALib-1.0
Start date:
17 Apr 2015
Due date:
% Done:

20%

Estimated time:
Spent time:
1.95 h

Description

The mem fn SparsePolyRingBase::myAddMul has strange semantics; the only documentation is a 1 line comment in the source code.

Who calls it? And why?

Can it be replaced by something "cleaner"?

History

#1 Updated by John Abbott about 9 years ago

I have just added a check that the first arg must be non-zero (o/w LM does not exist).
Should it be an error or a CoCoA_ASSERT?

#2 Updated by John Abbott about 9 years ago

Mario Albert calls myAddMul in a "bad" way in TmpJBEnv.C:323.
From his comments it seems he wants to do:

res += LM(h);
h -= LM(h);

How can we help him do this efficiently and clearly?

#3 Updated by Anna Maria Bigatti about 9 years ago

  • % Done changed from 0 to 10

John Abbott wrote:

Mario Albert calls myAddMul in a "bad" way in TmpJBEnv.C:323.
From his comments it seems he wants to do:
[...]

How can we help him do this efficiently and clearly?

I replace his code into

      RingElem tmp(myOptions.myRingOptions.myPolyRing);
      myOptions.myRingOptions.myPolyRing->myMoveLM(raw(tmp), raw(h));
      myOptions.myRingOptions.myPolyRing->myAppendClear(raw(res), raw(tmp));

which is more expressive and efficient.
This is what is done by myMoveToNextLM() as well. Still not very nice though.

The function myMoveLM moves into the top position of polynomial f1 the top summand from polynomial f2.
What ReductionCog needs is a function that moves into the last position of polynomial f1 the top summand from polynomial f2. I expect this is the same Mario wants to do: .... well, no test fails ;-)

Maybe I should rename myMoveLM into myMoveLMtoLM and make myMoveLMtoTail?
That would be more expressive on what it does (and warn to user of the dangers!)

#4 Updated by John Abbott about 9 years ago

  • Status changed from New to In Progress
  • Assignee set to Anna Maria Bigatti
  • % Done changed from 10 to 20
Better names:
  • myMoveLMToFront
  • myMoveLMToBack
  • myAddMulLM

#5 Updated by Anna Maria Bigatti about 9 years ago

  • Status changed from In Progress to New
  • % Done changed from 20 to 10

#6 Updated by Anna Maria Bigatti about 9 years ago

  • % Done changed from 10 to 20

Also available in: Atom PDF