FiniteRankFreeModule: Move all module identifications to methods exterior_power, dual_exterior_power, tensor_module

[Matthias Köppe]

Currently, the identification of a base module M with M.tensor_module(1, 0) and M.exterior_power(1) is implemented in FiniteRankFreeModule.__init__ but the identification of M.base_ring() with M.exterior_power(0) is implemented in exterior_power.

In this ticket, we move all identifications to the methods tensor_module, exterior_power, dual_exterior_power.

We also rewrite these methods in try/except style.

[Matthias Köppe]

New commits:

​1bd50f8	FiniteRankFreeModule: Move all module identifications to methods exterior_power, dual_exterior_power, tensor_module; Rewrite in try/except style
​d64c8ed	ExtPowerFreeModule._an_element_: Make sure a default basis is available
​d2d7f42	ExtPowerDualFreeModule._an_element_: Make sure a default basis is available
​9e739e5	FreeModuleLinearGroup._an_element_: Make sure a default basis is available
​eec19fd	TensorFreeModule._an_element_: Make sure a default basis is available
​1378d06	Merge branch 't/30254/tensorfreemodule__an_element___create_a_default_basis_in_the_base_module_if_necessary' into t/30255/finiterankfreemodule__move_all_module_identifications_to_methods_exterior_power__dual_exterior_power__tensor_module
​5a340d9	TensorFreeModule.__init__: Remove duplicate registration of self in the base module, uniqueness check
​de35db3	FreeModuleBasis.__init__: Update all modules in the set fmodule._all_modules
​2ac26d7	Merge branch 't/30250/finiterankfreemodule__simplify_unique_representation_code_for_dependent_modules' into t/30255/finiterankfreemodule__move_all_module_identifications_to_methods_exterior_power__dual_exterior_power__tensor_module

[Matthias Köppe]

Ha! This one did not pass the sage.manifolds doctests. Hold on...

[git]

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

​a6961c6

VectorFieldFreeModule: Update methods exterior_power, dual_exterior_power, tensor_module for changed FiniteRankFreeModule.__init__

[Matthias Köppe]

The code duplication between these three methods of FiniteRankFreeModule and VectorFieldFreeModule could of course be removed by using class attributes _exterior_power_class, _exterior_dual_power_class, _tensor_module_class. I can do that on a follow-up ticket if you like.

[Eric Gourgoulhon]

Thanks for this cleaning.

What is the advantage of

try:
    return d[k]
except KeyError:
    d[k] = ...
    return d[k]

over

if k not in d:
    d[k] = ...
return d[k]

? The latter looks easier to read.

[Matthias Köppe]

It's more than twice as fast in the fast path.

before:

sage: sage: M = FiniteRankFreeModule(ZZ, 3, name='M')
sage: sage: M.exterior_power(2)
2nd exterior power of the Rank-3 free module M over the Integer Ring
sage: %timeit M.exterior_power(2)
The slowest run took 9.54 times longer than the fastest. This could mean that an intermediate result is being cached.
100000 loops, best of 5: 2.23 µs per loop

after:

sage: sage: M = FiniteRankFreeModule(ZZ, 3, name='M')
sage: sage: M.exterior_power(2)
2nd exterior power of the Rank-3 free module M over the Integer Ring
sage: %timeit M.exterior_power(2)
The slowest run took 13.09 times longer than the fastest. This could mean that an intermediate result is being cached.
1000000 loops, best of 5: 929 ns per loop

[Travis Scrimshaw]

How does it compare in the slow path?

[Matthias Köppe]

The slow path is 2 orders of magnitude slower than the fast path because a heavy weight object is created:

sage: M = FiniteRankFreeModule(ZZ, 3, name='M')
sage: %time M.exterior_power(1)
CPU times: user 729 µs, sys: 692 µs, total: 1.42 ms
Wall time: 891 µs
Rank-3 free module M over the Integer Ring
sage: %time M.exterior_power(2)
CPU times: user 685 µs, sys: 18 µs, total: 703 µs
Wall time: 707 µs
2nd exterior power of the Rank-3 free module M over the Integer Ring
sage: %time M.exterior_power(3)
CPU times: user 239 µs, sys: 11 µs, total: 250 µs
Wall time: 254 µs
3rd exterior power of the Rank-3 free module M over the Integer Ring
sage: %time M.exterior_power(4)
CPU times: user 229 µs, sys: 0 ns, total: 229 µs
Wall time: 233 µs
4th exterior power of the Rank-3 free module M over the Integer Ring
sage: %time M.exterior_power(5)
CPU times: user 269 µs, sys: 14 µs, total: 283 µs
Wall time: 303 µs
5th exterior power of the Rank-3 free module M over the Integer Ring
sage: %time M.exterior_power(0)
CPU times: user 15 µs, sys: 1 µs, total: 16 µs
Wall time: 16.7 µs
Integer Ring

So whatever the changes to this method do does not matter for the slow path.

[Eric Gourgoulhon]

Replying to mkoeppe:

It's more than twice as fast in the fast path.

Thanks for the explanation.

[Matthias Köppe]

Ready for review

[Michael Jung]

Very nice changes.

First, the import in the doctest is obsolete now:

- sage: from sage.tensor.modules.tensor_free_module import TensorFreeModule

More comments follow...

[Michael Jung]

No okay, that's all. LGTM.

[Michael Jung]

Sorry. I'll give positive review when the line is removed.

[git]

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

​c035d06

Remove unused import in doctest

[Michael Jung]

LGTM. Patchbot is now full green, too.

[Matthias Köppe]

Thanks!