Project

General

Profile

Design #1065

Design: should we allow an ideal to change ring?

Added by Anna Maria Bigatti about 1 year ago. Updated 8 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Data Structures
Target version:
Start date:
11 May 2017
Due date:
% Done:

100%

Estimated time:
2.20 h
Spent time:

Description

I have some trouble writing a bit of code because I cannot redefine an ideal in another ring (chenging characteristic).
Should we allow that, as we did for RingElem?
Probably not...


Related issues

Related to CoCoALib - Bug #1064: Bug in MinPolyModular (ugly prime)Closed2017-05-10

History

#1 Updated by Anna Maria Bigatti about 1 year ago

This would be convenient, but also potentially dangerous:

In the case of RingElem the empty constructor gives 0 in ZZ, and this is mathematically (but not automatically) mapped into any other ring.

Similarly the empty ideal constructor should give the ideal 0 in ZZ, but this is not quite the same situation.

Having many rings around is delicate. I'm very undecided.

#2 Updated by Anna Maria Bigatti about 1 year ago

  • Related to Bug #1064: Bug in MinPolyModular (ugly prime) added

#3 Updated by Anna Maria Bigatti about 1 year ago

I managed to write my loop changing rings cleanly (modular computation).
While changing characteristic I forcily had a common ground (in my case the original ring P over QQ), so the check for the loop condition could be in P, keeping the changing ideal within the loop body.
This is indeed cleaner design!!

Moral: I have been forced to think to work around it, and the result was cleaner code.

#4 Updated by John Abbott about 1 year ago

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

The students here working on translating HomomorphismFns.cpkg5 into CoCoALib wanted to reassign an ideal changing ring. The code was something like this:

ideal I;
if (IsPolyRing(R)) { I = ideal(zero(R)); }
else { /*R is QuotientRing*/ I = DefiningIdeal(R); }

The above code causes problems for CoCoALib because currently an ideal variable may contain ideals only from the ring of the initial value given to that variable.

The simple solution (allowing the students to follow the structure of the CoCoA-5 package) would be to allow ideal variables to be assigned any ideal, i.e. without any restriction on the ring to which the ideal must belong.

If we maintain the restriction then the code must be reorganized (presumably by splitting the single fn into several functions, one for each subcase).

Personally I do not see any advantage in keeping the restriction on assigning ideals; it seems to make CoCoALib awkward to use without giving any (obvious) benefit.

#5 Updated by Anna Maria Bigatti about 1 year ago

John Abbott wrote:

The students here working on translating HomomorphismFns.cpkg5 into CoCoALib wanted to reassign an ideal changing ring. The code was something like this:
[...]

a quick trick I used in similar cases is to make an external function:

ideal IdealFromRing(const ring& R)
{
  if (IsPolyRing(R)) return ideal(zero(R));
 /*R is QuotientRing*/ return DefiningIdeal(R); 
}

Anyway I agree that, after allowing a RingElem to change ring, we can do the same for ideals.

#6 Updated by John Abbott about 1 year ago

  • % Done changed from 10 to 20

An important point in Anna's previous comment is that her function is a "trick".
It seems unreasonable to require the user to write an "ad hoc" global function (presumably in an anonymous namespace?) to achieve something which should be trivial.

#7 Updated by John Abbott 12 months ago

  • Status changed from In Progress to Feedback
  • Assignee set to John Abbott
  • Target version changed from CoCoALib-0.99999 to CoCoALib-0.99560
  • % Done changed from 20 to 90

I have implemented the change: just delete the check inside operator= for ideals.
Some simple tests passed (e.g. the students with their code).
So I'm putting this in feedback.

#8 Updated by John Abbott 8 months ago

  • Status changed from Feedback to Closed
  • % Done changed from 90 to 100

#9 Updated by John Abbott 8 months ago

  • Estimated time set to 2.20 h

Also available in: Atom PDF