## Design #1065

### Design: should we allow an ideal to change ring?

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 Bigattiabout 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 Bigattiabout 1 year ago

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

#### #3 Updated by Anna Maria Bigattiabout 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 Abbottabout 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 Bigattiabout 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 Abbottabout 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 Abbott12 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 Abbott8 months ago

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

#### #9 Updated by John Abbott8 months ago

• Estimated time set to 2.20 h

Also available in: Atom PDF