Opened 16 months ago
Closed 16 months ago
#28331 closed defect (fixed)
AttributeError when computing manifold chart after computing its vector field module
Reported by:  slelievre  Owned by:  

Priority:  minor  Milestone:  sage8.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 py2based or py3based, on Cygwin, Debian, macOS.
Change History (15)
comment:1 Changed 16 months ago by
comment:2 Changed 16 months ago by
 Priority changed from major to minor
comment:3 followup: ↓ 4 Changed 16 months ago by
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
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 followups: ↓ 6 ↓ 7 Changed 16 months ago by
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
, reusing 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 ; followup: ↓ 8 Changed 16 months ago by
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 AttributeError
s from public methods is still a bug no matter what, albeit a minor one in this case :)
comment:7 in reply to: ↑ 5 ; followup: ↓ 9 Changed 16 months ago by
Replying to slelievre:
One possible explanation to the observed error would be that when running
sage t src/sage/manifolds/differentiable/vectorfield_module.pythe doctest runner skipped the "repeated" line
sage: M = Manifold(2, M)in the doctests for
zero
, reusing theM
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
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
AttributeError
s 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
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
InterestingI 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
 Branch set to public/manifolds/vector_field_module_error_message28331
 Commit set to 3cefd361a5e9193768578215097c18472fce854d
New commits:
3cefd36  Better error messages related to nonfree vector field modules

comment:12 Changed 16 months ago by
 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 2dimensional differentiable manifold M has already been constructed as a nonfree module, which implies that the 2dimensional 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 2dimensional differentiable manifold M has already been constructed as a nonfree module and therefore cannot have a basis
comment:13 followup: ↓ 14 Changed 16 months ago by
 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
comment:15 Changed 16 months ago by
 Branch changed from public/manifolds/vector_field_module_error_message28331 to 3cefd361a5e9193768578215097c18472fce854d
 Resolution set to fixed
 Status changed from positive_review to closed
This error results from incompatible statements from the user. The first one is
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 C^{oo}(M) that is not free, since this is the generic case. But then the second statement:
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 C^{oo}(M). Hence the incompatibility.
If one reverses the order of the two statements, then everything is fine:
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
toTrue
: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.