Project

General

Profile

Design #1085

Fns with "OUT" args: should they give ERR::MixedRings?

Added by John Abbott almost 7 years ago. Updated 28 days ago.

Status:
Closed
Priority:
High
Assignee:
John Abbott
Category:
Safety
Target version:
CoCoALib-0.99850
Start date:
30 Jun 2017
Due date:
% Done:

100%

Estimated time:
1.11 h
Spent time:
1.15 h

Description

The functions IsDivisible (with 3 args) and GCDQuot have OUT args.
They throw ERR::MixedRings if any of the OUT params belongs to another ring.
Is this reasonable/correct?

Discuss.

This outline example will give a MixedRings error (assuming P is not ZZ)

   RingElem f(P);
   RingElem g(P);
   ...
   RingElem quot; // in ZZ by default
   if (!IsDivisible(quot, f,g)) CoCoA_ERROR(...);

Related issues

Related to CoCoA-5 - Feature #7: Automatic mapping between (some) ringsResolved2011-10-20

Related to CoCoALib - Design #1500: IsDivisible in a field?Closed2020-10-05

History

#1 Updated by John Abbott almost 7 years ago

  • Status changed from New to In Progress
  • Assignee set to John Abbott
  • % Done changed from 0 to 10

I would expect the following two code excerpts to be essentially equivalent:

RingElem quot; // 0 in ZZ
bool DoesDivide = IsDivisible(f,g);
if (!DoesDivide) { ... }
quot = f/g; // wasteful recomputation


RingElem quot; // 0 in ZZ
if (!IsDivisible(quot, f,g)) { ... }

However, the second, neater approach currently gives an error if the ring of quot is wrong. This design was probably inspired by the old idea of not letting RingElem variables change ring.

#2 Updated by John Abbott almost 7 years ago

I wonder which other fns are affected?
Presumably we should decide the same way for all of them...

When I have time, I shall search through the source code to see which other fns must be checked.
  • GcdQuot (done, I think)
  • operator+= etc. (in ring.C)

#3 Updated by John Abbott over 6 years ago

  • Target version changed from CoCoALib-0.99560 to CoCoALib-0.99600

#4 Updated by Anna Maria Bigatti over 5 years ago

  • Target version changed from CoCoALib-0.99600 to CoCoALib-0.99650 November 2019

#5 Updated by John Abbott over 4 years ago

  • Target version changed from CoCoALib-0.99650 November 2019 to CoCoALib-0.99700

#6 Updated by John Abbott over 4 years ago

  • Target version changed from CoCoALib-0.99700 to CoCoALib-0.99800

#7 Updated by John Abbott almost 4 years ago

  • Related to Feature #7: Automatic mapping between (some) rings added

#8 Updated by John Abbott over 3 years ago

#9 Updated by John Abbott over 3 years ago

  • % Done changed from 10 to 20

This issue is a generalized version of issue #1500 (which itself is concerned with the example in comment 1 of this issue).

My current thinking is that the ring to which an OUT parameter belongs should now be ignored, and the result simply put into the correct ring (and no MixedRings exception thrown).

Hopefully there are not too many fns with OUT params (of RingElem type). It seems that I was creating a list in comment 2 above.

#10 Updated by John Abbott over 3 years ago

  • % Done changed from 20 to 30

#11 Updated by John Abbott about 2 years ago

  • Target version changed from CoCoALib-0.99800 to CoCoALib-0.99850

#12 Updated by John Abbott about 1 month ago

Verbal discussion: do not give MixedRings error. So that the two code excerpts in comment 1 are equivalent.
JAA will check through all cases... (yawn!)

#13 Updated by John Abbott about 1 month ago

  • Status changed from In Progress to Feedback
  • % Done changed from 30 to 90

This seems to have been done already, at least mostly. So it has morally been in feedback for several month, at least.
Should we include some tests? Well, we should; but who has any desire to do so?

#14 Updated by John Abbott 28 days ago

  • Status changed from Feedback to Closed
  • % Done changed from 90 to 100
  • Estimated time set to 1.11 h

I'm too lazy to track down all the functions which ought to be tested, and then to write the corresponding tests.
So closing without writing any tests... if we get burned later then we can write some tests (Microsoft approach).

Also available in: Atom PDF