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:

Status badges

Description (last modified by caravantes)

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 caravantes

  • Branch set to u/caravantes/bott_bundles_2nd_attempt

comment:2 Changed 7 years ago by git

  • Commit set to a24cf5225fffbc2020eaf64f244e4ea5fb88a418

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

a24cf52Added package bott_bundles

comment:3 Changed 7 years ago by caravantes

  • Description modified (diff)
  • Status changed from new to needs_review

comment:4 follow-up: Changed 7 years ago by chapoton

  • 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 caravantes

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

Thanks!

comment:6 follow-up: Changed 7 years ago by chapoton

EXAMPLES: should be EXAMPLES::

TODO: should be .. TODO::

comment:7 Changed 7 years ago by git

  • Commit changed from a24cf5225fffbc2020eaf64f244e4ea5fb88a418 to ee8b1c6d2dccca7f2de20d7c51df982662360357

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

ee8b1c6Modified bott_bundles.py to add doctests to all functions lacking.

comment:8 in reply to: ↑ 6 Changed 7 years ago by caravantes

  • 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/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

comment:9 follow-up: Changed 7 years ago by chapoton

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 ; follow-up: Changed 7 years ago by caravantes

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 6 years ago by git

  • 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 6 years ago by caravantes

Mistypes should be corrected now. Needing review again.

comment:13 follow-ups: Changed 6 years ago by 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.

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#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)

comment:14 in reply to: ↑ 13 Changed 6 years ago by caravantes

Thanks. We'll try to rework it.

comment:15 Changed 6 years ago by caravantes

  • Status changed from needs_review to needs_work

comment:16 Changed 6 years ago by git

  • Commit changed from d12715b44d9db3120682d46964d98468b1e8ae39 to 6c83c184c562fc75cf3049f8b961f634f9820e7f

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

comment:17 Changed 6 years ago by git

  • Commit changed from 6c83c184c562fc75cf3049f8b961f634f9820e7f to d3a226c9e5ec4a53121ae55d669cfb8f92a61eec

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

d3a226cNow all classes inherit from SageObject

comment:18 in reply to: ↑ 13 Changed 6 years ago by caravantes

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 caravantes

  • Status changed from needs_work to needs_review

comment:20 follow-up: Changed 6 years ago by vdelecroix

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 caravantes

  • Authors set to Jorge Caravantes and Alicia Tocino

comment:22 in reply to: ↑ 20 Changed 6 years ago by caravantes

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! Done

Last edited 6 years ago by caravantes (previous) (diff)

comment:23 Changed 5 years ago by vdelecroix

  • Authors changed from Jorge Caravantes and Alicia Tocino to Jorge Caravantes, Alicia Tocino

(just modifying the Authors field)

comment:24 Changed 4 years ago by nbruin

  • Status changed from needs_review to needs_work
  • Work issues set to merge conflicts
Note: See TracTickets for help on using tickets.