#28331 closed defect (fixed)

AttributeError when computing manifold chart after computing its vector field module

Reported by: slelievre Owned by:
Priority: minor Milestone: sage-8.9
Component: geometry Keywords: manifolds
Cc: egourgoulhon, embray Merged in:
Authors: Eric Gourgoulhon Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: 3cefd36 (Commits) Commit: 3cefd361a5e9193768578215097c18472fce854d
Dependencies: Stopgaps:

Description

The following produces an attribute error:

sage: M = Manifold(2, 'M')
sage: XM = M.vector_field_module()
sage: X.<x, y> = M.chart()
Traceback (most recent call last)
...
AttributeError: 'VectorFieldModule_with_category' object has no attribute '_known_bases'

Observed with Sage 8.9.beta5, whether py2-based or py3-based, on Cygwin, Debian, macOS.

Change History (15)

comment:1 Changed 16 months ago by egourgoulhon

This error results from incompatible statements from the user. The first one is

sage: XM = M.vector_field_module()

When asked to construct the module of vector fields just after the manifold declaration, in absence of any further information, Sage returns a module over Coo(M) that is not free, since this is the generic case. But then the second statement:

sage: X.<x, y> = M.chart()

implies that M is parallelizable, since it can be covered by a single chart. The set of vector fields must then be a free module over Coo(M). Hence the incompatibility.

If one reverses the order of the two statements, then everything is fine:

sage: M = Manifold(2, 'M')
sage: X.<x, y> = M.chart()
sage: XM = M.vector_field_module()
sage: XM
Free module X(M) of vector fields on the 2-dimensional differentiable manifold M
sage: XM.category()
Category of finite dimensional modules over Algebra of differentiable scalar fields
 on the 2-dimensional differentiable manifold M

Indeed, when asked for the module of vector fields, Sage knows that it must be a free module, the manifold being manifestly parallelizable.

If one would like to keep the original order, i.e. create the module of vector fields before the chart, one must set the optional argument force_free to True:

sage: M = Manifold(2, 'M')
sage: XM = M.vector_field_module(force_free=True)
sage: X.<x, y> = M.chart()
sage: XM
Free module X(M) of vector fields on the 2-dimensional differentiable manifold M

So I would say that to avoid the AttributeError, one shall return an explicit error message when trying to create a chart, the domain of which is assumed to be not parallelizable.

comment:2 Changed 16 months ago by egourgoulhon

  • Priority changed from major to minor

comment:3 follow-up: Changed 16 months ago by embray

Hi Eric, thanks for the explanation that makes complete sense.

To provide further context to this ticket, no one actually intentionally tried to do things in the order reported. Rather, we were investigating an unrelated issue where, for some reason, it seems some doctests were being run out of order and/or skipping statements. So it just happened that this was run by the doctest runner (even though there's no test that actually does this intentionally).

One could claim "garbage in, garbage out". However, it still seemed like a real bug that we were getting a seemingly random AttributeError in this case.

I agree with your assessment that the best thing to do here would be to catch this case and raise a more useful exception.

comment:4 in reply to: ↑ 3 Changed 16 months ago by egourgoulhon

Replying to embray:

Hi Eric, thanks for the explanation that makes complete sense.

To provide further context to this ticket, no one actually intentionally tried to do things in the order reported. Rather, we were investigating an unrelated issue where, for some reason, it seems some doctests were being run out of order and/or skipping statements. So it just happened that this was run by the doctest runner (even though there's no test that actually does this intentionally).

One could claim "garbage in, garbage out". However, it still seemed like a real bug that we were getting a seemingly random AttributeError in this case.

Agreed.

I agree with your assessment that the best thing to do here would be to catch this case and raise a more useful exception.

OK I'll prepare this.

comment:5 follow-ups: Changed 16 months ago by slelievre

To explain further, last week I ran make ptestlong on Cygwin on Windows 7 for the first time (on SageMath 8.9.beta5 built from source there). It gave a dozen of failures not observed on other systems.

One of these failures seemed to boil down to the present example.

In src/sage/manifolds/differentiable/vectorfield_module.py we find the class VectorFieldModule with its many methods including identity_map and zero.

The method identity_map has tests starting at line 1003:

sage: M = Manifold(2, M)
sage: XM = M.vector_field_module()
sage: X.identity_map()

The method zero comes next, with tests starting at line 1023:

sage: M = Manifold(2, M)
sage: X.<x,y> = M.chart()  # makes M parallelizable
sage: XM = M.vector_field_module()
sage: XM.zero()

One possible explanation to the observed error would be that when running

sage -t src/sage/manifolds/differentiable/vectorfield_module.py

the doctest runner skipped the "repeated" line

sage: M = Manifold(2, M)

in the doctests for zero, re-using the M defined using an identical line in the previous doctest.

If we figure out exactly what goes wrong, we'll open a targeted ticket.

Sorry for the extra work here, maybe no user would ever have typed that.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 16 months ago by embray

Replying to slelievre:

Sorry for the extra work here, maybe no user would ever have typed that.

Maybe, maybe not. I could see it being an easy mistake to make, and throwing unhandled AttributeErrors from public methods is still a bug no matter what, albeit a minor one in this case :)

comment:7 in reply to: ↑ 5 ; follow-up: Changed 16 months ago by egourgoulhon

Replying to slelievre:

One possible explanation to the observed error would be that when running

sage -t src/sage/manifolds/differentiable/vectorfield_module.py

the doctest runner skipped the "repeated" line

sage: M = Manifold(2, M)

in the doctests for zero, re-using the M defined using an identical line in the previous doctest.

Why should the doctest runner skip this line? Manifold has *not* the unique representation property: each call to it, even with the same arguments, generates a different object. In other words, in any Sage session, we have

sage: Manifold(2, 'M') is Manifold(2, 'M')
False

comment:8 in reply to: ↑ 6 Changed 16 months ago by egourgoulhon

Replying to embray:

Replying to slelievre:

Sorry for the extra work here, maybe no user would ever have typed that.

Maybe, maybe not. I could see it being an easy mistake to make, and throwing unhandled AttributeErrors from public methods is still a bug no matter what, albeit a minor one in this case :)

Agreed. I'll fix this.

comment:9 in reply to: ↑ 7 Changed 16 months ago by egourgoulhon

Replying to egourgoulhon:

Why should the doctest runner skip this line? Manifold has *not* the unique representation property: each call to it, even with the same arguments, generates a different object.

Well actually, there may be a possible explanation: the function Manifold returns objects that belong to classes (like DifferentiableManifold) inheriting from UniqueRepresentation. It is creating them by using the argument unique_tag=getrandbits(128)*time() in DifferentiableManifold.__init__, where getrandbits is defined in sage.misc.prandom (see line 2650 in src.sage.manifolds.manifold.py). So in principle, each created object should be unique. If for some reason, the doctest runner returns twice the same value for getrandbits(128)*time() (very fast machine + unexpected behavior of getrandbits ??), then this could explain the observed behavior...

The reason why manifold classes inherit from UniqueRepresentation is to make pickling work easily, which is required for multithreading parallelization. By working on the pickling, maybe we could get rid of UniqueRepresentation (keeping only WithEqualityById) and of the unique_tag argument.

comment:10 Changed 16 months ago by embray

Interesting--I never would have just guessed that. I haven't had a chance yet to look into what was going on with the test runner on that machine. So far all we have to go on is the observed behavior, though it's not behavior I've seen before.

It's entirely possible that what you wrote here is relevant in this particular case. I can look into it next time Samuel and I are together.

comment:11 Changed 16 months ago by egourgoulhon

  • Branch set to public/manifolds/vector_field_module_error_message-28331
  • Commit set to 3cefd361a5e9193768578215097c18472fce854d

New commits:

3cefd36Better error messages related to non-free vector field modules

comment:12 Changed 16 months ago by egourgoulhon

  • Authors set to Eric Gourgoulhon
  • Status changed from new to needs_review

With the above commit, a ValueError is now returned, with a more explicit message:

sage: M = Manifold(2, 'M')
sage: XM = M.vector_field_module()
sage: X.<x, y> = M.chart()
Traceback (most recent call last):
...
ValueError: the Module X(M) of vector fields on the 2-dimensional
 differentiable manifold M has already been constructed as a
 non-free module, which implies that the 2-dimensional
 differentiable manifold M is not parallelizable and hence cannot
 be the domain of a coordinate chart

Similarly, if one attempts to construct a vector frame, while XM is not a free module, one now gets

sage: e = M.vector_frame('e')
Traceback (most recent call last):
...
ValueError: the Module X(M) of vector fields on the 2-dimensional
 differentiable manifold M has already been constructed as a
 non-free module and therefore cannot have a basis

comment:13 follow-up: Changed 16 months ago by embray

  • Reviewers set to Erik Bray
  • Status changed from needs_review to positive_review

Looks great, thank you.

comment:14 in reply to: ↑ 13 Changed 16 months ago by egourgoulhon

Replying to embray:

Looks great, thank you.

Thanks for the review!

comment:15 Changed 16 months ago by vbraun

  • Branch changed from public/manifolds/vector_field_module_error_message-28331 to 3cefd361a5e9193768578215097c18472fce854d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.