sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.44k stars 480 forks source link

Bott Bundles, second attempt #18468

Open 22c4e851-4968-4ae0-a06a-003b74a9a7fa opened 9 years ago

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 9 years ago

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.

Component: algebraic geometry

Work Issues: merge conflicts

Author: Jorge Caravantes, Alicia Tocino

Branch/Commit: u/caravantes/bott_bundles_2nd_attempt @ d3a226c

Issue created by migration from https://trac.sagemath.org/ticket/18468

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 9 years ago

Branch: u/caravantes/bott_bundles_2nd_attempt

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

a24cf52Added package bott_bundles
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Commit: a24cf52

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 9 years ago

Description changed:

--- 
+++ 
@@ -1 +1,5 @@
+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.
fchapoton commented 9 years ago
comment:4

according to the patchbot:

Missing doctests geometry/bott_bundles.py 33 / 36 = 91%

You need to doctest every single function to reach 100% coverage

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 9 years ago
comment:5

Thought I had. I'll try to do it.

Thanks!

fchapoton commented 9 years ago
comment:6

EXAMPLES: should be EXAMPLES::

TODO: should be .. TODO::

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

ee8b1c6Modified bott_bundles.py to add doctests to all functions lacking.
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from a24cf52 to ee8b1c6

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 9 years ago
comment:8

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/sage-download-file", line 418, in <module>
    tarball = Tarball(url)
  File "/home/jorge/sage/src/bin/sage-download-file", line 265, in __init__
    self.base, self.verson, self.ext = self._parse_name(tarball_name)
  File "/home/jorge/sage/src/bin/sage-download-file", line 290, in _parse_name
    raise ValueError('tarball name must be of the form source-x.y.z.[tar.*|zip|tgz]')
ValueError: tarball name must be of the form source-x.y.z.[tar.*|zip|tgz]
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.yamagata-u.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://mirrors-ru.go-parts.com/sage/sagemath/
  251ms: http://mirrors-uk.go-parts.com/sage/sagemath/
  273ms: http://mirrors-usa.go-parts.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://www-ftp.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
fchapoton commented 9 years ago
comment:9

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

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 9 years ago
comment:10

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.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

Changed commit from ee8b1c6 to d12715b

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 9 years ago

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

d12715b modified: src/sage/geometry/bott_bundles.py
22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 9 years ago
comment:12

Mistypes should be corrected now. Needing review again.

vbraun commented 9 years ago
comment:13

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

Style guide issue: INPUT has two dashes between name and description http://doc.sagemath.org/html/en/developer/coding_basics.html#the-docstring-of-a-function-content

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)

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 9 years ago
comment:14

Thanks. We'll try to rework it.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from d12715b to 6c83c18

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

4f54f23Added package bott_bundles
83e4fd9Modified bott_bundles.py to add doctests to all functions lacking.
d77503b modified: src/sage/geometry/bott_bundles.py
6d381d8Updated to sage 7.1. Referee comments applied
e9caab8Merge branch 'u/caravantes/bott_bundles_2nd_attempt' of trac.sagemath.org:sage into bott_bundles
6c83c18Removed src/sage/geometry/bott_bundles.py
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

Changed commit from 6c83c18 to d3a226c

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 8 years ago

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

d3a226cNow all classes inherit from SageObject
22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 8 years ago
comment:18

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.

videlec commented 8 years ago
comment:20

Please add your full name in the "Authors" field of the ticket (otherwise the patchbot will not care about it).

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 8 years ago

Author: Jorge Caravantes and Alicia Tocino

22c4e851-4968-4ae0-a06a-003b74a9a7fa commented 8 years ago
comment:22

Replying to @videlec:

Please add your full name in the "Authors" field of the ticket (otherwise the patchbot will not care about it).

¡Thanks for the indication! Done

videlec commented 8 years ago

Changed author from Jorge Caravantes and Alicia Tocino to Jorge Caravantes, Alicia Tocino

videlec commented 8 years ago
comment:23

(just modifying the Authors field)

nbruin commented 6 years ago

Work Issues: merge conflicts