Opened 14 months ago

Last modified 3 months ago

#31713 needs_work defect

Ring not considered module over itself

Reported by: gh-mjungmath Owned by:
Priority: major Milestone: sage-9.7
Component: categories Keywords:
Cc: tscrim, mkoeppe, dimpase, slelievre, gh-DaveWitteMorris Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: u/gh-mjungmath/ring_not_considered_module_over_itself (Commits, GitHub, GitLab) Commit: 0ef2060f1057f32de55d864954240fb930e3e879
Dependencies: #31721 Stopgaps:

Status badges

Description

We have the following bug:

sage: P = QQ[x]
sage: P in Modules(QQ)
True
sage: P in Modules(P)
False
sage: Modules(P)(P)
---------------------------------------------------------------------------
NotImplementedError                       Traceback (most recent call last)
...
NotImplementedError:

but on the other hand

sage: P^1 in Modules(P)
True

works.

Change History (28)

comment:1 Changed 14 months ago by slelievre

Something to document rather than to fix, if you ask me.

Similarly, a field is not in the category of vector spaces even though it is a vector space over itself.

sage: QQ in VectorSpaces(QQ)
False
sage: QQ^1 in VectorSpaces(QQ)
True

comment:2 Changed 14 months ago by gh-mjungmath

I would like to define a morphisms in the category of modules from a ring to a module over that ring. But now we get:

sage: Hom(QQ, QQ^2, category=Modules(QQ))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
ValueError: Rational Field is not in Category of vector spaces over Rational Field

Any way to circumvent this then?

Similarly for the dual btw.

Last edited 14 months ago by gh-mjungmath (previous) (diff)

comment:3 follow-up: Changed 14 months ago by tscrim

I don't consider this to be a bug, but a consequence of how we often secretly equate things in mathematics. QQ doesn't know it is a 1-dimensional module because it doesn't know how to do linear algebra (albeit it is trivial). So I also believe it should be documented and does not need a fix.

It seems like you just need to be a little more careful with what objects you are working with if you want it to be a module morphism (rather than an action).

comment:4 in reply to: ↑ 3 ; follow-up: Changed 14 months ago by gh-mjungmath

Replying to tscrim:

I don't consider this to be a bug, but a consequence of how we often secretly equate things in mathematics. QQ doesn't know it is a 1-dimensional module because it doesn't know how to do linear algebra (albeit it is trivial). So I also believe it should be documented and does not need a fix.

It seems like you just need to be a little more careful with what objects you are working with if you want it to be a module morphism (rather than an action).

But that doesn't answer my question how I can define a morphism (i.e. module homomorphism) from a ring to a module over the same ring. This must be somehow possible in Sage.

Fine, this works with QQ when you convert it into a vector space via QQ^1 but this trick doesn't work for e.g. scalar fields anymore.

Last edited 14 months ago by gh-mjungmath (previous) (diff)

comment:5 follow-up: Changed 14 months ago by slelievre

Can you turn the scalar field with scalars in some ring R into a vector field with vectors in R^1?

comment:6 in reply to: ↑ 5 Changed 14 months ago by gh-mjungmath

Replying to slelievre:

Can you turn the scalar field with scalars in some ring R into a vector field with vectors in R^1?

As far as I know, you can't, unfortunately. And I don't think there is a need to change anything or add this features. The only thing I need is a module homomorphism starting/ending from/to the scalar field algebra. It's really annoying that this is not working...

Last edited 14 months ago by gh-mjungmath (previous) (diff)

comment:7 Changed 14 months ago by slelievre

Despite my initial comment, I'm fine with making each ring a module over itself.

(Given how little I know about categories in Sage, I should listen more than speak.)

comment:8 in reply to: ↑ 4 Changed 14 months ago by tscrim

Replying to gh-mjungmath:

Replying to tscrim:

I don't consider this to be a bug, but a consequence of how we often secretly equate things in mathematics. QQ doesn't know it is a 1-dimensional module because it doesn't know how to do linear algebra (albeit it is trivial). So I also believe it should be documented and does not need a fix.

It seems like you just need to be a little more careful with what objects you are working with if you want it to be a module morphism (rather than an action).

But that doesn't answer my question how I can define a morphism (i.e. module homomorphism) from a ring to a module over the same ring. This must be somehow possible in Sage.

If you want a more hack way around it, you can modify the __contains__ for the modules. However, I don't like equating things implicitly. QQ does not know it is a 1-dimensional module over itself; it doesn't have that structure. In particular, you do not have the guarantee of certain methods being implemented. This will cause massive problems when you start using it as a module.

Again, if you want a module morphism, then you need to construct it as a module. Otherwise you have to have a ring morphism (which is what is used to define the action of QQ on QQ-modules).

Fine, this works with QQ when you convert it into a vector space via QQ^1 but this trick doesn't work for e.g. scalar fields anymore.

That sounds more like a problem with the implementation of scalar fields because the free module code should work with any field as linear algebra works over any field. Note that QQ^1 is just syntactic sugar for FreeModule(QQ, 1).

comment:9 Changed 14 months ago by gh-mjungmath

This makes total sense.

What about a coercion that exactly does this, coercing a ring R to FreeModule(R, 1) under the category coercion Modules(R)(R)?

comment:10 Changed 14 months ago by gh-mjungmath

  • Branch set to u/gh-mjungmath/ring_not_considered_module_over_itself

comment:11 Changed 14 months ago by gh-mjungmath

  • Commit set to a815b7195824da6a14541e83ad505120094ec417

I have uploaded a draft for the coercion. Please take a look.


New commits:

a815b71Trac #31713: coerce rings into modules

comment:12 Changed 14 months ago by git

  • Commit changed from a815b7195824da6a14541e83ad505120094ec417 to e68b8a42a95c67deaec3e9eeace7a1aa219f1577

Branch pushed to git repo; I updated commit sha1. New commits:

e68b8a4Trac #31713: map=False to make vector space coercion work

comment:13 Changed 14 months ago by git

  • Commit changed from e68b8a42a95c67deaec3e9eeace7a1aa219f1577 to 0ef2060f1057f32de55d864954240fb930e3e879

Branch pushed to git repo; I updated commit sha1. New commits:

0ef2060Trac #31713: unify to map=False

comment:14 Changed 14 months ago by gh-mjungmath

  • Status changed from new to needs_review

The last step, set map=False, was necessary to make Modules(QQ)(QQ) work in cases where free_module or vector_space do not provide the optional argument map. In addition, this unifies the global behavior of free_module and vector_space methods due to the principle: if you want more, you have to state more.

Last edited 14 months ago by gh-mjungmath (previous) (diff)

comment:15 Changed 14 months ago by gh-mjungmath

  • Status changed from needs_review to needs_work

I have another idea. I think, we need a __pow__ method for general parents that delegate to _pow_ in the categories. So far, the method _pow_int in categories.fields has no purpose at all.

Then we deal with the coercion via self**1.

Last edited 14 months ago by gh-mjungmath (previous) (diff)

comment:16 Changed 14 months ago by gh-mjungmath

  • Dependencies set to #31721

comment:17 follow-ups: Changed 14 months ago by tscrim

To be very clear about this, this is not a coercion, but a conversion. I also don't think we should simply check that x in _Rings because what if x is not a subring of the base ring? I think we should explicitly check that x is self.base() or x in self.base() (the latter being when the base is a category).

Changing the default behavior of free_module is a regression that will break code in the wild and requires at least a deprecation message.

I don't like perma-nailing categories into memory. This should be avoided as much as possible. I highly doubt the calls to those categories in free_module.py are a source of slowness. If they already are nailed in memory, you should import from there. I am also think we should keep using the is_foo() functions there too unless you have a good reason to change this in this ticket.

comment:18 in reply to: ↑ 17 ; follow-ups: Changed 14 months ago by gh-mjungmath

Replying to tscrim:

I don't like perma-nailing categories into memory. This should be avoided as much as possible. I highly doubt the calls to those categories in free_module.py are a source of slowness. If they already are nailed in memory, you should import from there. I am also think we should keep using the is_foo() functions there too unless you have a good reason to change this in this ticket.

The reason is that for example

sage: M = Manifold(2, 'M')
sage: A = M.scalar_field_algebra()
sage: FreeModule(A, 1)

won't work otherwise. Rings that do not inherit from ring.pyx don't admit the necessary methods. Perhaps it is a better idea to move all those methods to the category...?

As for perma-nailing, that wasn't my idea. It happens everywhere in the code.

comment:19 in reply to: ↑ 18 Changed 14 months ago by nbruin

Replying to gh-mjungmath:

As for perma-nailing, that wasn't my idea. It happens everywhere in the code.

One should indeed try to avoid instantiating instanced of parametrized categories (such as categories that have a base ring and/or dimensions) because they can lead to unbounded memory use over time.

See #15801. There are good solutions for that: Rather than Modules(QQ) use Modules(Fields()). The base ring can be obtained from the object itself anyway, so it doesn't need to be stored on its category. Note that objects can have their category "refined", so Modules(QQ) can still be instantiated if absolutely wanted by the user, and then an object can be made to have that category. The system shouldn't instantiate those categories uninvited, though. This has caused real computational failures in CRT-based computations (there are a lot of finite fields ...), so this is not some theoretical problem.

It may well be that subsequent developers were not aware of the real issues addressed in #15801 and, inspired by a desire for mathematical precision, have reintroduced automatic instantiation of parametrized categories. These are bugs that should be corrected.

comment:20 in reply to: ↑ 18 ; follow-up: Changed 14 months ago by tscrim

Replying to gh-mjungmath:

Replying to tscrim:

I don't like perma-nailing categories into memory. This should be avoided as much as possible. I highly doubt the calls to those categories in free_module.py are a source of slowness. If they already are nailed in memory, you should import from there. I am also think we should keep using the is_foo() functions there too unless you have a good reason to change this in this ticket.

The reason is that for example

sage: M = Manifold(2, 'M')
sage: A = M.scalar_field_algebra()
sage: FreeModule(A, 1)

won't work otherwise. Rings that do not inherit from ring.pyx don't admit the necessary methods. Perhaps it is a better idea to move all those methods to the category...?

Then add these methods to make it work as it suggests an incomplete implementation.

As for perma-nailing, that wasn't my idea. It happens everywhere in the code.

Then import it from those locations because if we decide to stop nailing it in, then we only have to change it in one place and then fix the errors in all of the files. Do not nail anything more than what already is done. Just because it is done elsewhere does not make it a good practice.

comment:21 in reply to: ↑ 20 Changed 14 months ago by gh-mjungmath

Replying to tscrim:

Replying to gh-mjungmath:

Replying to tscrim:

I don't like perma-nailing categories into memory. This should be avoided as much as possible. I highly doubt the calls to those categories in free_module.py are a source of slowness. If they already are nailed in memory, you should import from there. I am also think we should keep using the is_foo() functions there too unless you have a good reason to change this in this ticket.

The reason is that for example

sage: M = Manifold(2, 'M')
sage: A = M.scalar_field_algebra()
sage: FreeModule(A, 1)

won't work otherwise. Rings that do not inherit from ring.pyx don't admit the necessary methods. Perhaps it is a better idea to move all those methods to the category...?

Then add these methods to make it work as it suggests an incomplete implementation.

Either way: the category Rings must be refactored then. If a ring must have these methods, this must be stated in the category, e.g. via abstract methods or halfway implementations. See #31722.

As for perma-nailing, that wasn't my idea. It happens everywhere in the code.

Then import it from those locations because if we decide to stop nailing it in, then we only have to change it in one place and then fix the errors in all of the files. Do not nail anything more than what already is done. Just because it is done elsewhere does not make it a good practice.

That can be done in #31722.

comment:22 Changed 14 months ago by tscrim

That does not follow. It is not a category implementation but a class that is using those parents. There is no refactoring needed nor documentation needed to be added there.

Now there is a good reason to nail Fields(). There are a lot of people (by my understanding) that use code that works over prime numbers as the prime (or its power) vary in tight loops. For this, the difference between a specific category instance and checking the cache matters. There might be some other things that require a similar treatment, but just doing it arbitrarily is not good.

comment:23 in reply to: ↑ 17 ; follow-up: Changed 14 months ago by nbruin

Replying to tscrim:

I don't like perma-nailing categories into memory. This should be avoided as much as possible. I highly doubt the calls to those categories in free_module.py are a source of slowness. If they already are nailed in memory, you should import from there. I am also think we should keep using the is_foo() functions there too unless you have a good reason to change this in this ticket.

Do you have a more objective reason than just taste? I agree that linking categories willy-nilly is not a good idea, but for the most part I don't think it's going to cause memory leaks: if I recall correctly, most categories are immortal once they get created anyway. So the leak is creating them in the first place :-).

For categories like Fields(), though, this is certainly not an issue: they're fundamentally immortal. Perhaps the namespace clutter of binding them to _Fields somewhere isn't so nice, but I don't think it will have a large memory impact: it's just one binding (each time) in the name space dictionary of a module. A method likely has a much more severe memory impact, because it needs to get bound for each object that gets instantiated.

It may well be that using R.is_fields() leads to better/more readable/more efficient code than R in _Fields, or that it's better to spell the latter as R in Fields() (even though it's necessarily a bit slower), but I don't think "perma nailing" by itself is bad here: it's just a binding to an object that exists anyway, much like the binding from ... import ... would create, or for that matter import ..., which binds a name in a namespace to a module that (has just been caused to) exist.

comment:24 in reply to: ↑ 23 ; follow-up: Changed 14 months ago by tscrim

Replying to nbruin:

Replying to tscrim:

I don't like perma-nailing categories into memory. This should be avoided as much as possible. I highly doubt the calls to those categories in free_module.py are a source of slowness. If they already are nailed in memory, you should import from there. I am also think we should keep using the is_foo() functions there too unless you have a good reason to change this in this ticket.

Do you have a more objective reason than just taste? I agree that linking categories willy-nilly is not a good idea, but for the most part I don't think it's going to cause memory leaks: if I recall correctly, most categories are immortal once they get created anyway. So the leak is creating them in the first place :-).

Indeed, categories should not be immortal. Although some of them might effectively be that way since we have standard rings and fields in Sage that nobody would suggest we do lazily (ZZ, QQ, etc.).

For categories like Fields(), though, this is certainly not an issue: they're fundamentally immortal. Perhaps the namespace clutter of binding them to _Fields somewhere isn't so nice, but I don't think it will have a large memory impact: it's just one binding (each time) in the name space dictionary of a module. A method likely has a much more severe memory impact, because it needs to get bound for each object that gets instantiated.

For the non-parameterized categories, this is less of an issue, but I don't want to allow more use of this without a good reason. Most importantly, I would want to do this in one place and one place only. Everything else should just be an import of that specific instance.

It may well be that using R.is_fields() leads to better/more readable/more efficient code than R in _Fields, or that it's better to spell the latter as R in Fields() (even though it's necessarily a bit slower), but I don't think "perma nailing" by itself is bad here: it's just a binding to an object that exists anyway, much like the binding from ... import ... would create, or for that matter import ..., which binds a name in a namespace to a module that (has just been caused to) exist.

I agree generally, but I don't want this to do be done in many places and make it seem like it is a good practice. It makes it harder to refactor and other developers might start to think they can do this for any category they want, which I don't think we should advocate. Which they then move onto more parents being allowed. I know this is a slippery-slope argument, so it doesn't have much merit, but it is something I am a bit concerned about.

comment:25 in reply to: ↑ 24 Changed 14 months ago by nbruin

Replying to tscrim:

Indeed, categories should not be immortal. Although some of them might effectively be that way since we have standard rings and fields in Sage that nobody would suggest we do lazily (ZZ, QQ, etc.).

If I recall correctly, categories are fundamentally immortal once they participate in the dynamic classes that are derived from them (isn't this always?) The category hierarchy is just too involved to be reliably fully dynamic (in the sense that parts of it can be removed and garbage collected).

I suspect that this might be rather fundamental: once you participate in Python's MRO inheritance mechanism, you'd probably already get permanently nailed there. Python is definitely not designed with a non-monotonously growing class hierarchy in mind, so I expect that the caching and storing strategies put in place to implement it, prevent decreases (by "accidental design" I expect -- as a consequence of other design decisions made; not as an intentional design goal)

This was implicitly acknowledged in #15801.

We suffer from similar effects in the coercion network and the conversion caching: it's just very hard to keep a graph in such a way that you can efficiently find paths and force path independence of other properties along these paths (the coercion maps; unambiguous inheritance) and still allow for nodes to vanish from the graph as well - in particular if you'd like independence to hold through time (at least limited to a run) as well. [and another source -- caches that enforce global uniqueness :-)]

comment:26 Changed 12 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

Setting a new milestone for this ticket based on a cursory review.

comment:27 Changed 6 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6

comment:28 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.6 to sage-9.7
Note: See TracTickets for help on using tickets.