Opened 2 years ago

Closed 2 years ago

#30094 closed defect (fixed)

Basis-dependent isomorphism from FiniteRankFreeModule to an object in the category ModulesWithBasis

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.2
Component: categories Keywords:
Cc: Eric Gourgoulhon, Travis Scrimshaw, Michael Jung Merged in:
Authors: Matthias Koeppe, Michael Jung Reviewers: Michael Jung, Matthias Koeppe, Eric Gourgoulhon, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 8a800cc (Commits, GitHub, GitLab) Commit: 8a800cc0734fe2c0c9e4e201faf5b9435fdab78e
Dependencies: #30180 Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

(This ticket has been narrowed to the topic mainly discussed in the comments. The topic originally in title and ticket description has been moved to #30174)

There are multiple incompatible protocols for dealing with bases of a free module in Sage (#19346).

One such protocol is defined by the category ModulesWithBasis. CombinatorialFreeModule is one of the parent classes in this category.

Another such protocol is defined by the parent class FiniteRankFreeModule, which is in the category Modules. A FiniteRankFreeModule can be equipped with a finite list of bases (each represented by an instance of FreeModuleBasis), none of which are considered distinguished. FiniteRankFreeModules are unique parents; the operation of equipping a FiniteRankFreeModule does not change the identity of the parent.

We define a method FiniteRankFreeModule. isomorphism_with_fixed_basis that establishes, given a basis, an isomorphism with a CombinatorialFreeModule (and thus a parent in the category ModulesWithBasis.

Change History (68)

comment:1 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:2 Changed 2 years ago by Matthias Köppe

Also it seems various maps are missing.

sage: Q3.has_coerce_map_from(QQ^3)
False
sage: Q3((QQ^3).an_element())
ValueError: the None has not been defined on the 3-dimensional vector space over the Rational Field

comment:3 Changed 2 years ago by Matthias Köppe

Cc: Michael Jung added

comment:4 Changed 2 years ago by Travis Scrimshaw

These are incompatible objects since QQ^3 has a distinguished basis (the standard Euclidean basis) whereas Q3 does not. So there should not be a map between them.

My 2 cents for the error reported in the ticket is to first check that the object is not in the category. The other option is to lift the vector_space method up to the category of modules and then implement a change_ring method for Q3.

comment:5 in reply to:  4 ; Changed 2 years ago by Matthias Köppe

Replying to tscrim:

These are incompatible objects since QQ^3 has a distinguished basis (the standard Euclidean basis) whereas Q3 does not. So there should not be a map between them.

Shouldn't there be a map in one direction that forgets the distinguished basis?

comment:6 in reply to:  5 Changed 2 years ago by Travis Scrimshaw

Replying to mkoeppe:

Replying to tscrim:

These are incompatible objects since QQ^3 has a distinguished basis (the standard Euclidean basis) whereas Q3 does not. So there should not be a map between them.

Shouldn't there be a map in one direction that forgets the distinguished basis?

No, because where would you send the point (1,3,1) to? How would you know how it is expressed in one of the bases of Q3?

comment:7 Changed 2 years ago by Matthias Köppe

Ok, I feel we had this discussion before

comment:8 Changed 2 years ago by Matthias Köppe

Should FreeModuleBasis get a method that creates the identity map from self.free_module() to a free module with that basis?

comment:9 in reply to:  8 ; Changed 2 years ago by Michael Jung

Replying to mkoeppe:

Should FreeModuleBasis get a method that creates the identity map from self.free_module() to a free module with that basis?

No, I don't think it should. The identity map (like any other automorphism) is a basis independent object.

comment:10 in reply to:  9 ; Changed 2 years ago by Travis Scrimshaw

Replying to gh-mjungmath:

Replying to mkoeppe:

Should FreeModuleBasis get a method that creates the identity map from self.free_module() to a free module with that basis?

No, I don't think it should. The identity map (like any other automorphism) is a basis independent object.

I might even go further to say that it would not be the identity map, but an isomorphism of free modules. We have to be careful about distinguishing things up to equality and up to isomorphism. For example, if I have V = span_R{x,y,z}, this is not equal to W = R^3. So there is no identity map from V -> W because I am not looking at End(V), but Mor(V, W). Yes, they are isomorphic, but there is no canonical isomorphism until I choose a distinguished basis for V and W.

comment:11 in reply to:  10 Changed 2 years ago by Matthias Köppe

Replying to tscrim:

Replying to gh-mjungmath:

Replying to mkoeppe:

Should FreeModuleBasis get a method that creates the identity map from self.free_module() to a free module with that basis?

No, I don't think it should. The identity map (like any other automorphism) is a basis independent object.

I might even go further to say that it would not be the identity map, but an isomorphism of free modules.

Of course, that's what I meant.

comment:12 in reply to:  10 ; Changed 2 years ago by Matthias Köppe

Replying to tscrim:

Replying to mkoeppe:

Should FreeModuleBasis get a method that creates the identity map from self.free_module() to a free module with that basis?

We have to be careful about distinguishing things up to equality and up to isomorphism. For example, if I have V = span_R{x,y,z}, this is not equal to W = R^3. ... there is no canonical isomorphism until I choose a distinguished basis for V and W.

In my question above, this would be a method of FreeModuleBasis - so there is a particular basis. And RR^3 also has a distinguished basis.

Last edited 2 years ago by Matthias Köppe (previous) (diff)

comment:13 in reply to:  12 ; Changed 2 years ago by Travis Scrimshaw

Replying to mkoeppe:

Replying to tscrim:

Replying to mkoeppe:

Should FreeModuleBasis get a method that creates the identity map from self.free_module() to a free module with that basis?

We have to be careful about distinguishing things up to equality and up to isomorphism. For example, if I have V = span_R{x,y,z}, this is not equal to W = R^3. ... there is no canonical isomorphism until I choose a distinguished basis for V and W.

In my question above, this would be a method of FreeModuleBasis - so there is a particular basis. And RR^3 also has a distinguished basis.

You were asking for a map from self.free_module(), which is the free module without a distinguished basis. If you want to go from a FreeModuleBasis, then that might be a good method as there is a canonical isomorphism of the homspace with the endspace and an identity map. Although I am not sure how much the basis is intended as a basis as a set or the span of the basis elements. I suspect the former, so such a method would not make sense. I would have to see what Eric and Michal say.

comment:14 in reply to:  13 ; Changed 2 years ago by Eric Gourgoulhon

Replying to tscrim:

Replying to mkoeppe:

Replying to tscrim:

Replying to mkoeppe:

Should FreeModuleBasis get a method that creates the identity map from self.free_module() to a free module with that basis?

We have to be careful about distinguishing things up to equality and up to isomorphism. For example, if I have V = span_R{x,y,z}, this is not equal to W = R^3. ... there is no canonical isomorphism until I choose a distinguished basis for V and W.

In my question above, this would be a method of FreeModuleBasis - so there is a particular basis. And RR^3 also has a distinguished basis.

You were asking for a map from self.free_module(), which is the free module without a distinguished basis. If you want to go from a FreeModuleBasis, then that might be a good method as there is a canonical isomorphism of the homspace with the endspace and an identity map. Although I am not sure how much the basis is intended as a basis as a set or the span of the basis elements. I suspect the former, so such a method would not make sense. I would have to see what Eric and Michal say.

I of course agree that one may implement a method in FreeModuleBasis that returns an isomorphism between Q3 and QQ^3, but for which purpose? Such an isomorphism cannot be used for a coercion since a coercion must be unique and cannot depend on a extra parameter such as a basis-dependent isomorphism.

Coming back to the issue mentioned in the ticket title and description, I would say that the problem arises because "vector spaces" as they are implemented in Sage are not really vector spaces in all their generality but only Cartesian powers F^n, where F is the base field.

comment:15 in reply to:  14 ; Changed 2 years ago by Matthias Köppe

Replying to egourgoulhon:

Coming back to the issue mentioned in the ticket title and description, I would say that the problem arises because "vector spaces" as they are implemented in Sage are not really vector spaces in all their generality but only Cartesian powers F^n, where F is the base field.

OK, I think we are getting closer.

When you say, '"vector spaces" as they are implemented in Sage', I assume you mean the ones implemented in sage.modules.free_module and constructed by the function VectorSpace, and then I agree with what you wrote.

What I was asking for in comment 8 is, given a basis, a map from a FiniteRankFreeModule (which is in the category of finite dimensional modules) to a parent that is in the category of finite dimensional modules with basis.

comment:16 in reply to:  15 ; Changed 2 years ago by Michael Jung

Replying to tscrim:

I might even go further to say that it would not be the identity map, but an isomorphism of free modules.

Well, yes. It is a special kind of automorphism. However, I wouldn't see it as an isomorphism between any two free modules of the same rank, in case this is what you are referring to.

Replying to tscrim:

Although I am not sure how much the basis is intended as a basis as a set or the span of the basis elements. I suspect the former, so such a method would not make sense. I would have to see what Eric and Michal say.

I am not sure what you mean with "the span of the basis elements". This does not make any sense to me because the span is always the free module again. Anyway, I would see an instance of FreeModuleBasis as a representation of a (finite) subset of the corresponding free module which is linearly independent and generates the module (possibly in the sense of formal linear combinations).

comment:17 in reply to:  4 Changed 2 years ago by Michael Jung

Replying to tscrim:

These are incompatible objects since QQ^3 has a distinguished basis (the standard Euclidean basis) whereas Q3 does not. So there should not be a map between them.

Agreed.

My 2 cents for the error reported in the ticket is to first check that the object is not in the category. The other option is to lift the vector_space method up to the category of modules and then implement a change_ring method for Q3.

Sounds good. Who goes for it?

comment:18 in reply to:  15 ; Changed 2 years ago by Eric Gourgoulhon

Replying to mkoeppe:

Replying to egourgoulhon:

Coming back to the issue mentioned in the ticket title and description, I would say that the problem arises because "vector spaces" as they are implemented in Sage are not really vector spaces in all their generality but only Cartesian powers F^n, where F is the base field.

OK, I think we are getting closer.

When you say, '"vector spaces" as they are implemented in Sage', I assume you mean the ones implemented in sage.modules.free_module and constructed by the function VectorSpace,

Yes.

and then I agree with what you wrote.

What I was asking for in comment 8 is, given a basis, a map from a FiniteRankFreeModule (which is in the category of finite dimensional modules) to a parent that is in the category of finite dimensional modules with basis.

Yes, this is the basis-dependent isomorphism I was referring to comment:14, but my concern, and that of Travis if I understand correctly, is that such an isomorphism cannot be used for a coercion, because it is not canonical (by definition, it depends on a choice of basis).

comment:19 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/unable_to_coerce_x___3_dimensional_vector_space_over_the_rational_field__into_category_of_vector_spaces_over_rational_field

comment:20 in reply to:  18 Changed 2 years ago by Matthias Köppe

Commit: 91e44f4479bfb30663873faa7de0467e542daebb

Replying to egourgoulhon:

Yes, this is the basis-dependent isomorphism I was referring to comment:14, but my concern, and that of Travis if I understand correctly, is that such an isomorphism cannot be used for a coercion, because it is not canonical (by definition, it depends on a choice of basis).

I've pushed a rough implementation of this to the ticket.

Sure, I don't need this to be a coercion. Just a module morphism.


New commits:

91e44f4sage.tensor.modules.FiniteRankFreeModule.basis_fixing_map: New

comment:21 in reply to:  16 ; Changed 2 years ago by Travis Scrimshaw

Replying to gh-mjungmath:

Replying to tscrim:

Although I am not sure how much the basis is intended as a basis as a set or the span of the basis elements. I suspect the former, so such a method would not make sense. I would have to see what Eric and Michal say.

I am not sure what you mean with "the span of the basis elements". This does not make any sense to me because the span is always the free module again. Anyway, I would see an instance of FreeModuleBasis as a representation of a (finite) subset of the corresponding free module which is linearly independent and generates the module (possibly in the sense of formal linear combinations).

The question is what does this class represent:

  1. The basis itself, which is a finite set.
  2. The span of the basis elements, which is a vector space where we can represent the points in terms of the chosen basis elements.

These are very different objects. One is finite, one is infinite (well, |R|n, where R is the base ring). Since you want to think of it as 1, then you are saying there should not be a morphism. However, most of the points of the free module should not be elements of this parent. The actual implementation is really treating it like 2, and its elements are the points of the free module represented using the chosen basis.


I like the proposed method overall. I have some suggestions:

1 - Change the name to something like morphism_with_fixed_basis as right now it sounds like it is constructing a map that happens to fix a basis. 2 - Allow the codomain of the map to be passed to the function. Then you can have the map be defined as

        if codomain.dimension() != self.dimension():
            raise ValueError
        B = list(codomain.basis())
        def function(x):
            return codomain.sum(x[basis, i] * B[i] for i in self.irange())

This should be implementation independent (if it is not, then it is a bug IMO).

comment:22 in reply to:  21 Changed 2 years ago by Matthias Köppe

Replying to tscrim:

Replying to gh-mjungmath:

Replying to tscrim:

Although I am not sure how much the basis is intended as a basis as a set or the span of the basis elements. I suspect the former, so such a method would not make sense. I would have to see what Eric and Michal say.

I am not sure what you mean with "the span of the basis elements". This does not make any sense to me because the span is always the free module again. Anyway, I would see an instance of FreeModuleBasis as a representation of a (finite) subset of the corresponding free module which is linearly independent and generates the module (possibly in the sense of formal linear combinations).

The question is what does this class represent:

  1. The basis itself, which is a finite set.
  2. The span of the basis elements, which is a vector space where we can represent the points in terms of the chosen basis elements.

I think it's clear that 1 is intended:

sage: V = FiniteRankFreeModule(QQ, 3)
sage: e = V.basis("e")
sage: e[0] in e
True
sage: e[1] in e
True
sage: e[0] + e[1] in e
False

comment:23 Changed 2 years ago by git

Commit: 91e44f4479bfb30663873faa7de0467e542daebb73667410fb02014b789a49ef58e9846ecc729419

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

7366741Rename, generalize

comment:24 Changed 2 years ago by Travis Scrimshaw

Ah, right. Yes, it is clear that it is 1. I got myself confused.

Last edited 2 years ago by Travis Scrimshaw (previous) (diff)

comment:25 in reply to:  23 ; Changed 2 years ago by Michael Jung

Replying to git:

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

7366741Rename, generalize

I must agree Eric in comment:14, I don't see the use of such a method isomorphism_with_fixed_basis, not even in FiniteRankFreeModule.

I see no problem in implementing the coercion on the level of modules right away. There is no need for such method either in this case. The vector space QQ^3 forgets about the basis we have chosen, anyway (i.e. there is no going back). The important thing is that we are able to choose a basis. But this is ensured due to the definition of a free module.

I have to admit, I am not sure what's the problem is we're still discussing; apart from the error reported in the description. Is it still all about comment:2? As reported in this ticket, we want to coerce the free modules themselves and not their elements.

Last edited 2 years ago by Michael Jung (previous) (diff)

comment:26 in reply to:  25 Changed 2 years ago by Matthias Köppe

Replying to gh-mjungmath:

I have to admit, I am not sure what's the problem is we're still discussing

Thanks. I will split the ticket into two and clarify the descriptions.

comment:27 Changed 2 years ago by Matthias Köppe

Description: modified (diff)
Summary: unable to coerce x (=3-dimensional vector space over the Rational Field) into Category of vector spaces over Rational FieldBasis-dependent isomorphism from FiniteRankFreeModule to an object in the category ModulesWithBasis

comment:28 in reply to:  25 Changed 2 years ago by Matthias Köppe

The topic originally on the ticket description is now in #30174. I hope this helps.

comment:29 Changed 2 years ago by Michael Jung

Looking at the code, I begin to like the idea, too. :)

It's still a draft, however I'd like to propose an improvement regarding the docstring:

         Construct the canonical isomorphism from the free module ``self``
-        to a free module in which ``basis`` of ``self`` is the distinguished basis.
+        to a free module in which ``basis`` of ``self`` is mapped to the
+        distinguished basis of ``codomain``.

I think it would also be good to specify input and output in the docstring. In particular, the input codomain should be described as an element of ModulesWithBasis having the same rank as self and over the same base ring. Furthermore, it should be explained that CombinatorialFreeModule is chosen by default if the input is None.

Last edited 2 years ago by Michael Jung (previous) (diff)

comment:30 Changed 2 years ago by Michael Jung

And probably we should raise an error in case both base rings are not equal.

comment:31 Changed 2 years ago by Matthias Köppe

Thanks for the suggestions, sounds all good. I don't have time to work on it today. Please feel free to commit changes to the branch

comment:32 Changed 2 years ago by Michael Jung

Branch: u/mkoeppe/unable_to_coerce_x___3_dimensional_vector_space_over_the_rational_field__into_category_of_vector_spaces_over_rational_fieldpublic/tensor/basis_dependent_isomorphism_free_modules
Commit: 73667410fb02014b789a49ef58e9846ecc729419

comment:33 Changed 2 years ago by git

Commit: 81e0ec0c1902a55269214f81e2cce130253b0de8

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

91e44f4sage.tensor.modules.FiniteRankFreeModule.basis_fixing_map: New
7366741Rename, generalize
81e0ec0Trac #30094: docstring improved + raise error for different base rings

comment:34 Changed 2 years ago by git

Commit: 81e0ec0c1902a55269214f81e2cce130253b0de8bdc6e2c8194911acf4e2a6a021e2f417504b9752

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

bdc6e2cTrac #30094: minor improvements in docstring

comment:35 Changed 2 years ago by Eric Gourgoulhon

Thanks for the code! I think this should be changed:

-            return codomain.sum(x[basis, i] * codomain_basis[i]
-                                for i in self.irange())
+            return codomain.sum(x[basis, i] * codomain_basis[i - self._sindex]
+                                for i in self.irange())

The doctest

sage: self = V = FiniteRankFreeModule(QQ, 3); V

could be changed to

sage: self = V = FiniteRankFreeModule(QQ, 3, start_index=1); V

to check that everything is OK with index ranges.

comment:36 Changed 2 years ago by git

Commit: bdc6e2c8194911acf4e2a6a021e2f417504b9752acaaa7fbb60c891ba80ecf504430a7c68c99dad9

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

acaaa7fTrac #30094: starting index corrected

comment:37 in reply to:  35 Changed 2 years ago by Michael Jung

Replying to egourgoulhon:

Thanks for the code! I think this should be changed:

-            return codomain.sum(x[basis, i] * codomain_basis[i]
-                                for i in self.irange())
+            return codomain.sum(x[basis, i] * codomain_basis[i - self._sindex]
+                                for i in self.irange())

The doctest

sage: self = V = FiniteRankFreeModule(QQ, 3); V

could be changed to

sage: self = V = FiniteRankFreeModule(QQ, 3, start_index=1); V

to check that everything is OK with index ranges.

Very good objection. I took that into consideration. I think, the ticket is ready for review now. Matthias?

comment:38 Changed 2 years ago by Matthias Köppe

Authors: Matthias Koeppe, Michael Jung
Status: newneeds_review

comment:39 Changed 2 years ago by Matthias Köppe

I'm happy with these changes, thanks very much!

comment:40 Changed 2 years ago by Matthias Köppe

Of course, one could ask what the basis of the CombinatorialFreeModule should be indexed by.

sage: V = FiniteRankFreeModule(QQ, 3, start_index=1)
sage: basis = e = V.basis("e")
sage: phi_e.codomain().basis()
Finite family {'e_1': B['e_1'], 'e_2': B['e_2'], 'e_3': B['e_3']}

I think it would be more natural if this family was indexed by V.irange().

comment:41 in reply to:  40 Changed 2 years ago by Michael Jung

Replying to mkoeppe:

Of course, one could ask what the basis of the CombinatorialFreeModule should be indexed by.

sage: V = FiniteRankFreeModule(QQ, 3, start_index=1)
sage: basis = e = V.basis("e")
sage: phi_e.codomain().basis()
Finite family {'e_1': B['e_1'], 'e_2': B['e_2'], 'e_3': B['e_3']}

I think it would be more natural if this family was indexed by V.irange().

I don't quite get what you want. I have quickly overviewd the documentation of CombinatorialFreeModule, and I think that our implementation already carries out the intended behavior.

comment:42 Changed 2 years ago by Matthias Köppe

sage: phi_e.codomain().basis().keys()
{'e_1', 'e_2', 'e_3'}

I think the code should be changed so that

sage: phi_e.codomain().basis().keys()
{1, 2, 3}

comment:43 in reply to:  42 Changed 2 years ago by Michael Jung

Replying to mkoeppe:

sage: phi_e.codomain().basis().keys()
{'e_1', 'e_2', 'e_3'}

I think the code should be changed so that

sage: phi_e.codomain().basis().keys()
{1, 2, 3}

As far as I understand it, the keys correspond to the basis element's name. And I thought that the intention was to name them after the basis we are coming from, wasn't it?

Notice that the names can also differ from the canonical choice, for example a, b, c, without labeled indices. In that case, the current implementation is preferrable anyway.

Last edited 2 years ago by Michael Jung (previous) (diff)

comment:44 Changed 2 years ago by Matthias Köppe

Are there methods in FreeModuleBasis or FiniteRankFreeModule that allow the user to look up a basis element by name?

comment:45 Changed 2 years ago by Michael Jung

I am sorry. I am little off track. You were obiously right:

basis_keys - list, tuple, family, set, etc. defining the indexing set for the basis of this module

What you suggest sounds good then.

comment:46 Changed 2 years ago by git

Commit: acaaa7fbb60c891ba80ecf504430a7c68c99dad981fa95dd8abee38ce70b57d35f1aa3c8667c7ffd

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

81fa95dTrac #30094: indexing improved

comment:47 Changed 2 years ago by git

Commit: 81fa95dd8abee38ce70b57d35f1aa3c8667c7ffd5657108b4d0ea40f08e79de42e1a7675dd2f2400

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

5657108Add an example with a provided codomain

comment:48 Changed 2 years ago by Matthias Köppe

Perhaps the prefix "B" should be changed to basis._symbol

comment:49 Changed 2 years ago by git

Commit: 5657108b4d0ea40f08e79de42e1a7675dd2f240040f589f9033607bae86eec49b1eeb2d020003880

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

40f589fUse basis._symbol as prefix instead of B

comment:50 in reply to:  49 Changed 2 years ago by Michael Jung

Replying to git:

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

40f589fUse basis._symbol as prefix instead of B

Replying to mkoeppe:

Perhaps the prefix "B" should be changed to basis._symbol

This won't work:

sage: M = FiniteRankFreeModule(QQ, 3)
sage: e = M.basis(['a', 'b', 'c'], symbol_dual=['d', 'e', 'f'])
sage: phi_e = phi_e = M.isomorphism_with_fixed_basis(e)
sage: phi_e.codomain().basis()
<repr(<sage.sets.family.FiniteFamily_with_category at 0x7f03d6b8bb88>) failed: TypeError: can only concatenate tuple (not "str") to tuple>

See comment:43.

Last edited 2 years ago by Michael Jung (previous) (diff)

comment:51 Changed 2 years ago by Michael Jung

Status: needs_reviewneeds_work

comment:52 Changed 2 years ago by git

Commit: 40f589f9033607bae86eec49b1eeb2d020003880affd0c974f144ca19f2095f3546130f04b188823

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

affd0c9Use basis._symbol as prefix only when it is a string

comment:53 Changed 2 years ago by Matthias Köppe

Status: needs_workneeds_review

Thanks for catching this. Here's a better version

comment:54 Changed 2 years ago by Michael Jung

I don't think the prefix is even necessary, but I can live with that.

comment:55 Changed 2 years ago by Matthias Köppe

Reviewers: Michael Jung, Matthias Koeppe, ...

comment:56 Changed 2 years ago by Eric Gourgoulhon

Reviewers: Michael Jung, Matthias Koeppe, ...Michael Jung, Matthias Koeppe, Eric Gourgoulhon
Status: needs_reviewpositive_review

LGTM. Thanks!

comment:57 Changed 2 years ago by Eric Gourgoulhon

Status: positive_reviewneeds_info

Shouldn't the code of this ticket use #30180 (cf. comment 1 in that ticket)? Then #30180 should be added as a dependency.

comment:58 in reply to:  57 ; Changed 2 years ago by Michael Jung

Replying to egourgoulhon:

Shouldn't the code of this ticket use #30180 (cf. comment 1 in that ticket)? Then #30180 should be added as a dependency.

I think, the independence should be the other way around, right Matthias?

comment:59 in reply to:  58 ; Changed 2 years ago by Eric Gourgoulhon

Replying to gh-mjungmath:

Replying to egourgoulhon:

Shouldn't the code of this ticket use #30180 (cf. comment 1 in that ticket)? Then #30180 should be added as a dependency.

I think, the independence should be the other way around, right Matthias?

No, if the code in this ticket makes use of #30180's code, the dependency shall be set here to #30180.

comment:60 in reply to:  59 Changed 2 years ago by Michael Jung

Replying to egourgoulhon:

Replying to gh-mjungmath:

Replying to egourgoulhon:

Shouldn't the code of this ticket use #30180 (cf. comment 1 in that ticket)? Then #30180 should be added as a dependency.

I think, the independence should be the other way around, right Matthias?

No, if the code in this ticket makes use of #30180's code, the dependency shall be set here to #30180.

This code works independently of #30180 and I don't see any snippets from #30180 used here.

comment:61 Changed 2 years ago by Matthias Köppe

Both of you are right.

comment:62 Changed 2 years ago by Matthias Köppe

Thanks to the speedy review of #30180, I will use it now to simplify the code on the present ticket.

comment:63 Changed 2 years ago by git

Commit: affd0c974f144ca19f2095f3546130f04b1888238a800cc0734fe2c0c9e4e201faf5b9435fdab78e

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

0afa01cModules: Add parent method module_morphism
3c8d080Fix docstring markup
e2fc5d5Make sure that we only create a module morphism
68da24cMerge branch 't/30180/category_modules_should_provide_a_parent_method_module_morphism_compatible_with_moduleswithbasis_module_morphism' into t/30094/public/tensor/basis_dependent_isomorphism_free_modules
8a800ccFiniteRankFreeModule.isomorphism_with_fixed_basis: Simplify using self.module_morphism

comment:64 Changed 2 years ago by Matthias Köppe

Dependencies: #30180
Status: needs_infoneeds_review

comment:65 Changed 2 years ago by Travis Scrimshaw

Morally green patchbot.

comment:66 Changed 2 years ago by Eric Gourgoulhon

Reviewers: Michael Jung, Matthias Koeppe, Eric GourgoulhonMichael Jung, Matthias Koeppe, Eric Gourgoulhon, Travis Scrimshaw
Status: needs_reviewpositive_review

Thanks for the update!

comment:67 Changed 2 years ago by Matthias Köppe

Thanks!

comment:68 Changed 2 years ago by Volker Braun

Branch: public/tensor/basis_dependent_isomorphism_free_modules8a800cc0734fe2c0c9e4e201faf5b9435fdab78e
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.