Open mkoeppe opened 1 year ago
hey i wanna work on this issue
Hi, seems like this issue is still not resolved. Can I work on this issue? @mkoeppe
Hi @mkoeppe. I have raised this PR for the issue. Can you please review the PR and let me know your thoughts on it?
I am -1 on the proposal. I don't think a name clash between two different objects is justification for changing the method name. The proposed name also might not make sense if we eventually support taking quotients. For instance, we could eventually support constructing a $K$-submodule $M$ inside an $L$-module $L^n$ for fields $K$ and $L$. Then if we build another $K$-submodule $N \subseteq M$ and took the quotient, the vectors still might need $L$ to describe themselves, but the quotient module would be ambient and over $K$.
I don't think a name clash between two different objects is justification for changing the method name.
Actually the motivation for resolving this name clash is so that we can have an object that is simultaneously a vector space and an algebraic scheme!
Okay, that wasn't clear from the proposal. I am also not sure they should be the same parent since their algebraic properties would be quite different I believe. IIRC, there was some serious thought put into the name coordinate_ring
for these types of module constructions. If you can find such discussions, it might be good to look over them.
If you can find such discussions
Well, that would be
As you can see there, the name coordinate_ring
was chosen based on elementary terminology from "vectors in R^n".
Unfortunately in this short discussion, nobody brought up that this name clashes with the substantial notion of coordinate rings of affine varieties --- which, of course, are natural generalizations of linear subspaces.
Edit: Cc'ing @nbruin @kwankyu in case there's interest
OK, this is for R a PID with K a field of fractions, and R-submodules M of K^n. In that case M.ambient() gives that vector space, so the result is M.ambient().base_ring(). That suggests ambient_base_ring()
is a fairly natural contraction, if that contraction is necessary at all.
I think you might run into problems if a you want objects that are simultaneously vector spaces and schemes. I could see a point set of a scheme admitting a vector space structure.
For discoverability, I would suggest base_ring_ambient()
, since it is really base_ring()
for most cases.
Internally, it is self.__coordinate_ring
. I think this should be changed together.
For discoverability, I would suggest
base_ring_ambient()
, since it is reallybase_ring()
for most cases.
Note that the naming is consistent with methods left_base_ring
, right_base_ring
of bimodules
Note that left_base_ring
and right_base_ring
have corresponding mathematical objects. There is none for ambient_base_ring
(the same for coordinate_ring
). This is a reason why I prefer base_ring_ambient
, which I read "base ring of the ambient space".
Note that
left_base_ring
andright_base_ring
have corresponding mathematical objects. There is none forambient_base_ring
(the same forcoordinate_ring
).
I'm not sure if I understand
In other words, I mean that "base ring of the ambient space" is more understandable than "ambient base ring".
Anyway I agree to change coordinate_ring()
to either one, whichever is okay with me.
Note also that there are plenty of hits in git grep 'def ambient_'
that use the same grammatical pattern as the proposed ambient_base_ring
in naming methods. I do understand the discoverability argument, but I think consistency is more important.
I still don't think ambient_base_ring()
is a future-proof name. If I take a quotient space of two $R$ subspaces of a $K$-vector space (here I am allowing $R$ to be any subfield of $K$, which is a fairly reasonable construction). This would be a free $R$-module with its at least one coordinate in $K \setminus R$ (generically). So ambient_base_ring()
doesn't make sense because the quotient does not have an ambient space (in the terminology/methods used in that code). The construction would be similar to:
sage: V = QQ^5
sage: M = V.subspace([[1,2,3,4,5],[2,3,4,5,6]])
sage: M
Vector space of degree 5 and dimension 2 over Rational Field
Basis matrix:
[ 1 0 -1 -2 -3]
[ 0 1 2 3 4]
sage: N = M.subspace([[1,2,3,4,5]])
sage: N
Vector space of degree 5 and dimension 1 over Rational Field
Basis matrix:
[1 2 3 4 5]
sage: Q = M / N
sage: Q.ambient_module() is Q
True
sage: Q.cover() is M
True
As another example, what if the output of the quotient below realized it was a free $\mathbb{Z}$-module and was the corresponding subclass?
sage: V = ZZ^5 * (1/2)
sage: M = V.submodule([[1/2,2,3,4,5],[2,3/2,4,5/2,6]])
sage: N = M.submodule([[1/2,2,3,4,5]])
sage: Q = M / N
I don't think the name clash is that major of an issue though (at least until we have schemes that are using this code and a conflict does arise). However, since there is a consensus to change it, I think we should be a bit more careful than at present. How about vector_coordinate_ring()
or vector_coefficient_ring()
?
@mkoeppe Okay, seems I was misremembering; less thought about the name, more about the concept. Thanks for finding those.
It is true that there are some peculiarities and overlaps in the terminology.
The notion of being "ambient" used in the classes of sage.modules
is clearly not the same as that in the Sets.Subquotients
protocol, where any parent can be the .ambient()
of a subquotient of it.
Why the quotient modules defined in sage.modules
consider themselves "ambient" is a bit of a mystery to me, and I think we should discuss whether that should be changed. In fact, in your first example,
sage: Q.category()
Category of finite dimensional vector spaces with basis over (number fields and quotient fields and metric spaces)
It does not even know that it is a quotient!
At the category level, yes. Perhaps because of the conflict. Or it predates the category (more likely).
Quotient modules are ambient because they are not a submodule of any other module. So this is clear (under that definition).
Perhaps at the category level, we should change ambient()
to cover()
. I don't think the quotients are used so often with ambient()
, but I have very limited data and anecdotes about that.
Another oddity/conflict, the lift()
method is based on lifting stuff from the base ring. This should have a more precise name, such as lift_coefficients()
.
Lastly, there is an unrelated concept of coordinate_module()
. This might not create such a direct conflict with schemes, but it is something else to consider.
Perhaps at the category level, we should change
ambient()
tocover()
. I don't think the quotients are used so often withambient()
, but I have very limited data and anecdotes about that.
Do you mean to change it to cover()
for quotients, but to keep it ambient()
for subobjects? The problem with that: subquotients.
How about
vector_coordinate_ring()
orvector_coefficient_ring()
?
No, I don't find the word "vector" helpful for clarifying anything there.
Edit: On second thought, it does of course relate to methods such as _vector_
, see #34487
I still don't think
ambient_base_ring()
is a future-proof name. If I take a quotient space of two R subspaces of a K-vector space (here I am allowing R to be any subfield of K, which is a fairly reasonable construction). This would be a free R-module with its at least one coordinate in K∖R (generically)
Once you mod out by an R-module that is not a K-vector space, I don't see what natural coordinates you'd choose on the quotient that take values in K. Are you thinking about still representing elements of the quotient by vectors in the K-vector space? I don't think a quotient module easily arises with an embedding, so I think it would arise as an abstract module with no ambient space.
I'm surprised we even have a class to keep track of modules over a PID with an embedding in a vector space over the field of fractions. I'd be even more surprised if that got generalized further. I think one would be able to keep track of this information just by keeping some appropriate homomorphisms around.
By the way, this ticket collects a number of related discussions and proposals:
I still don't think
ambient_base_ring()
is a future-proof name. If I take a quotient space of two R subspaces of a K-vector space (here I am allowing R to be any subfield of K, which is a fairly reasonable construction). This would be a free R-module with its at least one coordinate in K∖R (generically)Once you mod out by an R-module that is not a K-vector space, I don't see what natural coordinates you'd choose on the quotient that take values in K. Are you thinking about still representing elements of the quotient by vectors in the K-vector space? I don't think a quotient module easily arises with an embedding, so I think it would arise as an abstract module with no ambient space.
Abstractly I agree with you, and for modules, this makes no difference. However, when there is extra structure, such as a finite dimensional algebra quotiented by an ideal, then the most natural thing is to represent objects by the elements in the cover. This is useful to define the, e.g., multiplication. (Exactly like how we do for quotients of polynomial rings.)
I'm surprised we even have a class to keep track of modules over a PID with an embedding in a vector space over the field of fractions. I'd be even more surprised if that got generalized further. I think one would be able to keep track of this information just by keeping some appropriate homomorphisms around.
This just runs though fairly generic code if I understand correctly. I didn’t test that all the features/functionality works though. So things wrapping the free modules (which is what finite dimensional Lie algebras defined by structure coefficients does) would find it quite cumbersome to always have to convert between things with morphism.
Perhaps at the category level, we should change
ambient()
tocover()
. I don't think the quotients are used so often withambient()
, but I have very limited data and anecdotes about that.Do you mean to change it to
cover()
for quotients, but to keep itambient()
for subobjects? The problem with that: subquotients.
Yes, but I don’t see your point. It wouldn’t be unreasonable to have a subobject of a quotient to not have a cover, but there is a natural way to construct a cover I believe (by the natural lift of the basis). Please explain with an actual example and details.
This method is only defined for the free modules implemented in
sage.modules
, and it is incompatible with the method of the same name of algebraic schemes.We should rename it to remove this clash. A good replacement may be
ambient_base_ring
: