Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move most important attributes of FaceIterator to a structure. #28894

Closed
kliem opened this issue Dec 18, 2019 · 36 comments
Closed

Move most important attributes of FaceIterator to a structure. #28894

kliem opened this issue Dec 18, 2019 · 36 comments

Comments

@kliem
Copy link
Contributor

kliem commented Dec 18, 2019

This ticket is part of a series of tickets to parallelize the f-vector for polyhedra.

Python classes may not be accessed in nogil. To access important attributes of the class FaceIterator, we move those attributes to a structure, which can be accessed in nogil.

Along the way, the ticket simplifies some doctests that define cython functions.

CC: @jplab @LaisRast

Component: geometry

Keywords: combinatorial polyhedron, face iterator, f-vector

Author: Jonathan Kliem

Branch/Commit: 142d3a8

Reviewer: Jean-Philippe Labbé, Frédéric Chapoton

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

@kliem kliem added this to the sage-9.0 milestone Dec 18, 2019
@kliem
Copy link
Contributor Author

kliem commented Dec 18, 2019

New commits:

2582549important attributes of iterator in structure

@kliem
Copy link
Contributor Author

kliem commented Dec 18, 2019

Commit: 2582549

@kliem
Copy link
Contributor Author

kliem commented Dec 18, 2019

Branch: public/28894

@kliem
Copy link
Contributor Author

kliem commented Jan 6, 2020

Changed commit from 2582549 to db31390

@kliem
Copy link
Contributor Author

kliem commented Jan 6, 2020

Changed branch from public/28894 to public/28894-reb

@kliem
Copy link
Contributor Author

kliem commented Jan 6, 2020

New commits:

db31390important attributes of iterator in structure

@embray
Copy link
Contributor

embray commented Jan 6, 2020

comment:4

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-9.0, sage-9.1 Jan 6, 2020
@kliem
Copy link
Contributor Author

kliem commented Feb 14, 2020

New commits:

c3e2cb7important attributes of iterator in structure
3e7f7f8applied changes to simplicity and simpliciality

@kliem
Copy link
Contributor Author

kliem commented Feb 14, 2020

Changed commit from db31390 to 3e7f7f8

@kliem
Copy link
Contributor Author

kliem commented Feb 14, 2020

Changed branch from public/28894-reb to public/28894-reb2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 14, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8d933c3important attributes of iterator in structure
c0c9a31applied changes to simplicity and simpliciality

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 14, 2020

Changed commit from 3e7f7f8 to c0c9a31

@kliem
Copy link
Contributor Author

kliem commented Mar 19, 2020

comment:7

Apply failed.

@kliem
Copy link
Contributor Author

kliem commented Mar 19, 2020

New commits:

a58ffbeimportant attributes of iterator in structure
6d0644asimplification of doctests

@kliem
Copy link
Contributor Author

kliem commented Mar 19, 2020

Changed branch from public/28894-reb2 to public/28894-reb3

@kliem
Copy link
Contributor Author

kliem commented Mar 19, 2020

Changed commit from c0c9a31 to 6d0644a

@kliem

This comment has been minimized.

@mkoeppe
Copy link
Member

mkoeppe commented Apr 14, 2020

comment:9

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

@mkoeppe mkoeppe removed this from the sage-9.1 milestone Apr 14, 2020
@mkoeppe mkoeppe added this to the sage-9.2 milestone Apr 14, 2020
@kliem
Copy link
Contributor Author

kliem commented Apr 24, 2020

New commits:

41eb697important attributes of iterator in structure
56dcddfsrc/simplification of doctests

@kliem

This comment has been minimized.

@kliem
Copy link
Contributor Author

kliem commented Apr 24, 2020

Changed commit from 6d0644a to 56dcddf

@kliem
Copy link
Contributor Author

kliem commented Apr 24, 2020

Changed branch from public/28894-reb3 to public/28894-reb4

@kliem
Copy link
Contributor Author

kliem commented May 11, 2020

Changed commit from 56dcddf to 53fd2a2

@kliem
Copy link
Contributor Author

kliem commented May 11, 2020

Changed branch from public/28894-reb4 to public/28894-reb5

@kliem
Copy link
Contributor Author

kliem commented May 11, 2020

New commits:

0c44221important attributes of iterator in structure
efb0bd3src/simplification of doctests
53fd2a2fixed failing doctest

@jplab
Copy link

jplab commented May 11, 2020

Reviewer: Jean-Philippe Labbé

@jplab
Copy link

jplab commented May 11, 2020

comment:12

Looks good to me.

@kliem
Copy link
Contributor Author

kliem commented May 11, 2020

comment:13

Thank you.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 25, 2020

Changed commit from 53fd2a2 to 142d3a8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 25, 2020

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

142d3a8bad alignment causing bug noticed in #28982

@kliem
Copy link
Contributor Author

kliem commented May 25, 2020

comment:15

In the dual case the n_atoms/n_coatoms where initialized wrong in the face iterator. This made doctests in #28982 fail.

@fchapoton
Copy link
Contributor

Changed reviewer from Jean-Philippe Labbé to Jean-Philippe Labbé, Frédéric chapoton

@fchapoton
Copy link
Contributor

comment:16

back to positive

@fchapoton
Copy link
Contributor

Changed reviewer from Jean-Philippe Labbé, Frédéric chapoton to Jean-Philippe Labbé, Frédéric Chapoton

@kliem
Copy link
Contributor Author

kliem commented Jun 25, 2020

comment:18

Thank you.

@vbraun
Copy link
Member

vbraun commented Jun 27, 2020

Changed branch from public/28894-reb5 to 142d3a8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants