Opened 7 years ago
Last modified 4 years ago
#18468 needs_work enhancement
Bott Bundles, second attempt
Reported by:  caravantes  Owned by:  

Priority:  minor  Milestone:  
Component:  algebraic geometry  Keywords:  
Cc:  Merged in:  
Authors:  Jorge Caravantes, Alicia Tocino  Reviewers:  
Report Upstream:  N/A  Work issues:  merge conflicts 
Branch:  u/caravantes/bott_bundles_2nd_attempt (Commits, GitHub, GitLab)  Commit:  d3a226c9e5ec4a53121ae55d669cfb8f92a61eec 
Dependencies:  Stopgaps: 
Description (last modified by )
This ticket adds a package of name sage.geometry.bott_bundles to compute cohomology and rank of some homogeneous vector bundles over flag manifolds via Bott's theorem for the general linear group, see [w] in the documentation.
There is a lot of work to do on flag manifolds, but it seems it works quite well with grassmannians, while performing a Schur functor can be really slow.
We appreciate any review.
Change History (24)
comment:1 Changed 7 years ago by
 Branch set to u/caravantes/bott_bundles_2nd_attempt
comment:2 Changed 7 years ago by
 Commit set to a24cf5225fffbc2020eaf64f244e4ea5fb88a418
comment:3 Changed 7 years ago by
 Description modified (diff)
 Status changed from new to needs_review
comment:4 followup: ↓ 5 Changed 7 years ago by
 Status changed from needs_review to needs_work
according to the patchbot:
Missing doctests geometry/bott_bundles.py 33 / 36 = 91%
You need to doctest every single function to reach 100% coverage
comment:5 in reply to: ↑ 4 Changed 7 years ago by
Thought I had. I'll try to do it.
Thanks!
comment:6 followup: ↓ 8 Changed 7 years ago by
EXAMPLES:
should be EXAMPLES::
TODO:
should be .. TODO::
comment:7 Changed 7 years ago by
 Commit changed from a24cf5225fffbc2020eaf64f244e4ea5fb88a418 to ee8b1c6d2dccca7f2de20d7c51df982662360357
Branch pushed to git repo; I updated commit sha1. New commits:
ee8b1c6  Modified bott_bundles.py to add doctests to all functions lacking.

comment:8 in reply to: ↑ 6 Changed 7 years ago by
 Status changed from needs_work to needs_review
Hopefully this is now solved. Thanks for reviewing it.
I have tried to run patchbot, but I do not understand the output. However, I think I gave examples to all functions and when running doctests, everything apparently worked. I copy the output:
jorge@Grandote:~/sage$ sage patchbot Getting trusted author list... WARNING: Do not use this copy of sage while the patchbot is running. '/home/jorge/sage'/sage i ccache Attempting to download package ccache >>> Checking online list of optional packages. Traceback (most recent call last): File "/home/jorge/sage/src/bin/sagedownloadfile", line 418, in <module> tarball = Tarball(url) File "/home/jorge/sage/src/bin/sagedownloadfile", line 265, in __init__ self.base, self.verson, self.ext = self._parse_name(tarball_name) File "/home/jorge/sage/src/bin/sagedownloadfile", line 290, in _parse_name raise ValueError('tarball name must be of the form sourcex.y.z.[tar.*ziptgz]') ValueError: tarball name must be of the form sourcex.y.z.[tar.*ziptgz] Error: failed to download Downloading the Sage mirror list Searching fastest mirror 755ms: http://echidna.maths.usyd.edu.au/sage/ 512ms: http://files.sagemath.org/ 790ms: http://ftp.kaist.ac.kr/sage/ 462ms: http://ftp.leg.uct.ac.za/pub/packages/sage/ 1110ms: http://ftp.riken.jp/sagemath/ 1287ms: http://ftp.tsukuba.wide.ad.jp/software/sage/ 883ms: http://ftp.yz.yamagatau.ac.jp/pub/math/sage/ 893ms: http://jambu.spms.ntu.edu.sg/sage/ 1185ms: http://linorg.usp.br/sage/ 815ms: http://mirror.aarnet.edu.au/pub/sage/ 1376ms: http://mirror.hust.edu.cn/sagemath/ 560ms: http://mirror.ufs.ac.za/sagemath/ 214ms: http://mirror.yandex.ru/mirrors/sage.math.washington.edu/ 230ms: http://mirrorsru.goparts.com/sage/sagemath/ 251ms: http://mirrorsuk.goparts.com/sage/sagemath/ 273ms: http://mirrorsusa.goparts.com/sage/sagemath/ 272ms: http://mirrors.fe.up.pt/pub/sage/ 279ms: http://mirrors.mit.edu/sage/ 6516ms: http://mirrors.ustc.edu.cn/sagemath/ 531ms: http://mirrors.xmission.com/sage/ 399ms: http://sage.mirror.garr.it/mirrors/sage/ 11271ms: http://sagemath.c3sl.ufpr.br/ 948ms: http://sagemath.mirror.ac.za/ 1604ms: http://sagemath.polytechnic.edu.na/ 132ms: http://sunsite.rediris.es/mirror/sagemath/ 255ms: http://wwwftp.lip6.fr/pub/math/sagemath/ 916ms: http://www.cecm.sfu.ca/sage/ 264ms: http://www.mirrorservice.org/sites/www.sagemath.org/ Fastest mirror: http://sunsite.rediris.es/mirror/sagemath/ http://sunsite.rediris.es/mirror/sagemath//spkg/optional/list, aborting Traceback (most recent call last): File "/home/jorge/sage/local/bin/patchbot/patchbot.py", line 1091, in <module> main(args) File "/home/jorge/sage/local/bin/patchbot/patchbot.py", line 1031, in main do_or_die("'%s'/sage i ccache" % options.sage_root, exn_class=ConfigException) File "/home/jorge/sage/local/bin/patchbot/util.py", line 141, in do_or_die raise exn_class("%s %s" % (res, cmd)) util.ConfigException: 256 '/home/jorge/sage'/sage i ccache
comment:9 followup: ↓ 10 Changed 7 years ago by
Hello,
you do not need to run the patchbot, you can just have a look at the result of the existing patchbots, by clicking on the colored round icon at the top right of this page. This leads to the page where you can find the reports on your ticket. There is only one report so far because you are not a "trusted author" yet.
a few more things::
EXAPLES should be EXAMPLES
pletysm should be plethysm
The TODO:: should really be .. TODO::
with .. and space before
you can check coverage of a file by running sage coverage filename.py
comment:10 in reply to: ↑ 9 ; followup: ↓ 12 Changed 7 years ago by
Thanks again for all the info!
EXAPLES should be EXAMPLES pletysm should be plethysm
Ouch! Sorry!
The TODO:: should really be
.. TODO::
with .. and space before
Done, I hope.
you can check coverage of a file by running sage coverage filename.py
Thanks! It says 100% now.
I hope I can push during this week. Sorry for all the stupid mistakes and thanks for your patience and guidance.
comment:11 Changed 7 years ago by
 Commit changed from ee8b1c6d2dccca7f2de20d7c51df982662360357 to d12715b44d9db3120682d46964d98468b1e8ae39
Branch pushed to git repo; I updated commit sha1. New commits:
d12715b  modified: src/sage/geometry/bott_bundles.py

comment:12 in reply to: ↑ 10 Changed 7 years ago by
Mistypes should be corrected now. Needing review again.
comment:13 followups: ↓ 14 ↓ 18 Changed 6 years ago by
Since there is a lot more that one could do with flag manifolds I'd suggest a new directory sage/geometry/flag_manifold
and at least separate files for the base space and bundles.
Can you go through the comments and
 remove everything that is not needed:
# nu[j]=nu[j+1]1
and so on  remove debug print statements
 translate comments to English, I don't speak Spanish and have no idea what you mean
Style guide issue: INPUT has two dashes between name and description http://doc.sagemath.org/html/en/developer/coding_basics.html#thedocstringofafunctioncontent
INPUT:  ``p``  (default: 2) a positive prime integer.
Integration with the rest of Sage: object should at least inherit from SageObject and use _repr_
instead of __repr__
. There is a lot more, e.g. the semigroup structure on bundles etc but thats at least a start.
Constructors (and methods in general) should never print anything as side effect, otherwise you can't really use them as building blocks elsewhere. Only _repr_
should return a description. If you want a more detailed description, add a special method for that (explain()
or so).
Instead of .Q()
, consider more descriptive names like .universal_quotient_bundle()
. Tab completion will then be much more useful. Or collect them into an intermediate property like .bundle.universal_quotient()
(see also #15328)
comment:14 in reply to: ↑ 13 Changed 6 years ago by
Thanks. We'll try to rework it.
comment:15 Changed 6 years ago by
 Status changed from needs_review to needs_work
comment:16 Changed 6 years ago by
 Commit changed from d12715b44d9db3120682d46964d98468b1e8ae39 to 6c83c184c562fc75cf3049f8b961f634f9820e7f
Branch pushed to git repo; I updated commit sha1. New commits:
4f54f23  Added package bott_bundles

83e4fd9  Modified bott_bundles.py to add doctests to all functions lacking.

d77503b  modified: src/sage/geometry/bott_bundles.py

6d381d8  Updated to sage 7.1. Referee comments applied

e9caab8  Merge branch 'u/caravantes/bott_bundles_2nd_attempt' of trac.sagemath.org:sage into bott_bundles

6c83c18  Removed src/sage/geometry/bott_bundles.py

comment:17 Changed 6 years ago by
 Commit changed from 6c83c184c562fc75cf3049f8b961f634f9820e7f to d3a226c9e5ec4a53121ae55d669cfb8f92a61eec
Branch pushed to git repo; I updated commit sha1. New commits:
d3a226c  Now all classes inherit from SageObject

comment:18 in reply to: ↑ 13 Changed 6 years ago by
It's been a long time, but at last we could save some time to fix this. First of all, thanks for all the suggestions, and the patience to show us the correct way.
All comments shoud have been applied as suggested but the following one:
Replying to vbraun:
Since there is a lot more that one could do with flag manifolds I'd suggest a new directory
sage/geometry/flag_manifold
and at least separate files for the base space and bundles.
We have created the directory flag_manifold
and now moved the file inside it. It is now called bott_bundle.py
, since using singular instead of plural seems to be the convention.
We also have considered to include the file in schemes
, instead of geometry
, but it seeems that, while the dimension is easy to get, the partition that defines it as a flag manifold does not seem easy to extract from the object as a scheme
. Moreover, making it a child class from the flag manifolds class would require some changes in schemes
directory, and we are too afraid to change these files. Therefore, we still remain in geometry
.
In the end, we decided to leave the base spaces in bott_bunlde.py
and we have not created a flag_manifold.py
file. The main reason is that we could not manage to code methods like universal_quotient_bundle, since it seems that we need bott_bundle.py
calling flag_manifold.py
and viceversa. We can rework it if someone suggest us how to solve this problem of the double calling.
Instead of two separate files, we have renamed the base space classes to include ForBottBundles
. This way, names like Grassmannian
are free to use for different purposes.
As before, we appreciate any new comments.
comment:19 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:20 followup: ↓ 22 Changed 6 years ago by
Please add your full name in the "Authors" field of the ticket (otherwise the patchbot will not care about it).
comment:21 Changed 6 years ago by
comment:22 in reply to: ↑ 20 Changed 6 years ago by
Replying to vdelecroix:
Please add your full name in the "Authors" field of the ticket (otherwise the patchbot will not care about it).
¡Thanks for the indication!
comment:24 Changed 4 years ago by
 Status changed from needs_review to needs_work
 Work issues set to merge conflicts
Branch pushed to git repo; I updated commit sha1. New commits:
Added package bott_bundles