Opened 9 months ago
Last modified 3 months ago
#30241 new enhancement
New implementation class FiniteRankDualFreeModule
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.4 |
Component: | linear algebra | Keywords: | |
Cc: | egourgoulhon, tscrim, gh-mjungmath | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
(from #30169)
Currently, we have the following identifications:
sage: M = FiniteRankFreeModule(ZZ, 3, name='M') sage: M is M.exterior_power(1) True sage: M is M.tensor_module(1, 0) True
In contrast:
sage: M.dual() is M.dual_exterior_power(1) True sage: M.dual() is M.tensor_module(0, 1) False
The dual is implemented twice, as a special case of both TensorFreeModule
and ExtPowerDualFreeModule
.
We create a separate implementation class FiniteRankDualFreeModule
for it.
By adding __classcall_private__
methods, we delegate the construction to the new class for the special cases.
Change History (14)
comment:1 follow-up: ↓ 2 Changed 9 months ago by
comment:2 in reply to: ↑ 1 ; follow-up: ↓ 4 Changed 9 months ago by
Replying to gh-mjungmath:
This doesn't sound sensible to me.
ExtPowerDualFreeModule
andTensorFreeModule
follow very different construction patterns. For a reason: mathematically, meaning very strictly speaking, one forms and (0,1) tensors are defined differently, though they are isomorphic.
The same is true for M.exterior_power(1)
and M.tensor_module(1, 0)
, but we do have this identification.
comment:3 Changed 9 months ago by
By the way, I am adding the additional structure of the exterior powers as quotients of tensor modules in #30169. Please take a look
comment:4 in reply to: ↑ 2 ; follow-ups: ↓ 5 ↓ 6 ↓ 10 Changed 9 months ago by
Replying to mkoeppe:
Replying to gh-mjungmath:
This doesn't sound sensible to me.
ExtPowerDualFreeModule
andTensorFreeModule
follow very different construction patterns. For a reason: mathematically, meaning very strictly speaking, one forms and (0,1) tensors are defined differently, though they are isomorphic.The same is true for
M.exterior_power(1)
andM.tensor_module(1, 0)
, but we do have this identification.
Fair point. Then I would vote for changing this to the expected outputs rather than creating a whole new class which has no further purpose than everything that is already there. Regarding Travis comment:10 this would be a convenient solution. Meaning: M.exterior_power(1)
should return an instance of ExtPowerFreeModule
and M.tensor_module(1, 0)
should return an instance of TensorFreeModule
. Then, one can implement the isomorphisms.
Either way, I agree that consistency is desirable here.
Allow me a side note: please remember that the whole manifold setup is built upon FiniteRankFreeModule
. Modifying substatial things here might cause problems in the manifold implementation. It would be good to double check and, if absolutely necessary, make changes there, too. Henceforth, I suggest doctesting the manifold part, too.
comment:5 in reply to: ↑ 4 Changed 9 months ago by
Replying to gh-mjungmath:
... a whole new class which has no further purpose than everything that is already there.
Note that by creating the new class, both of the ExtPowerDualFreeModule
and TensorFreeModule
classes will be simplified because they no longer have to implement the special case.
comment:6 in reply to: ↑ 4 ; follow-up: ↓ 7 Changed 9 months ago by
Replying to gh-mjungmath:
M.exterior_power(1)
should return an instance ofExtPowerFreeModule
andM.tensor_module(1, 0)
should return an instance ofTensorFreeModule
.
Let's see. In your opinion, what should M.exterior_power(0)
and M.dual_exterior_power(0)
return?
comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed 9 months ago by
Replying to mkoeppe:
Replying to gh-mjungmath:
M.exterior_power(1)
should return an instance ofExtPowerFreeModule
andM.tensor_module(1, 0)
should return an instance ofTensorFreeModule
.Let's see. In your opinion, what should
M.exterior_power(0)
andM.dual_exterior_power(0)
return?
Strictly speaking, they should return instances of ExtPowerFreeModule
or ExtPowerDualFreeModule
respectively, which are isomorphic to the base ring.
You definitely have a good point. And I totally agree that this should be unified, in either way.
But I am strictly against a new class dedicated to just one special case. At least, this is how I understand your proposal.
comment:8 in reply to: ↑ 7 Changed 9 months ago by
Replying to gh-mjungmath:
I am strictly against a new class dedicated to just one special case.
I'll just prepare a branch and then you can see how it simplifies things, which will make it easier to review this proposal
comment:9 Changed 9 months ago by
Sure, I will take a look. Travis, what do you say?
comment:10 in reply to: ↑ 4 Changed 9 months ago by
Replying to gh-mjungmath:
Allow me a side note: please remember that the whole manifold setup is built upon
FiniteRankFreeModule
. Modifying substatial things here might cause problems in the manifold implementation. It would be good to double check and, if absolutely necessary, make changes there, too. Henceforth, I suggest doctesting the manifold part, too.
Thanks. Prompted by your remark, I have taken a look and I noticed that VectorFieldModule
also has similar code. I have yet to study this code in detail (I am slowly making my way up the stack...), but it looks it has the same inconsistency between primal and dual:
sage: M = Manifold(2, 'M') sage: XM = M.vector_field_module() sage: XM.tensor_module(1, 0) is XM True sage: XM.tensor_module(0, 1) is XM.dual() False
comment:11 follow-up: ↓ 12 Changed 9 months ago by
I think I have a solution that could make everyone happy.
In #30169, I had implemented FiniteRankDualFreeModule.__classcall_private__
methods that delegate to other classes and implement the identification.
But we could as well take care of all identifications in the FiniteRankModule.exterior_power
, dual_power
, tensor_module
methods.
Then the class constructors ExtPowerFreeModule
could keep the strict behavior (I would also check that the case of degree 0 is correctly implemented - I don't think it is currently).
comment:12 in reply to: ↑ 11 Changed 9 months ago by
Replying to mkoeppe:
I think I have a solution that could make everyone happy.
In #30169, I had implemented
FiniteRankDualFreeModule.__classcall_private__
methods that delegate to other classes and implement the identification.
But we could as well take care of all identifications in the
FiniteRankModule.exterior_power
,dual_power
,tensor_module
methods.
This sounds much better, yes. It would also be good to leave a note in the documentation so that the user knows about these special cases and how they are treated (if not already done).
Then the class constructors
ExtPowerFreeModule
could keep the strict behavior (I would also check that the case of degree 0 is correctly implemented - I don't think it is currently).
I would appreciate this task. The degree zero case had already caused some troubles in the past.
comment:13 Changed 9 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:14 Changed 3 months ago by
- Milestone changed from sage-9.3 to sage-9.4
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
This doesn't sound sensible to me.
ExtPowerDualFreeModule
andTensorFreeModule
follow very different construction patterns. For a reason: even from the mathematical point of view, one forms and (0,1) tensors are constructed differently, though they are isomorphic.From that perspective, the current implementation makes perfect sense. What would make more sense is implementing the isomorphism.