Opened 3 years ago
Closed 2 years ago
#29676 closed enhancement (fixed)
Make a nogil version of the most important methods of FaceIterator.
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  geometry  Keywords:  polytopes, fvector 
Cc:  JeanPhilippe Labbé, Laith Rastanawi, Travis Scrimshaw  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  81372b6 (Commits, GitHub, GitLab)  Commit:  81372b652d2dd92c87b80a0795bb276e49c67985 
Dependencies:  #28894, #29654  Stopgaps: 
Description (last modified by )
We outsource 3 crucial methods of FaceIterator
to nogil functions.
This is part of #28893: Parallel fvector for polyhedra.
Anything happening within prange
should be without gil. Any function that is called in the parallel section needs to be nogil.
Change History (12)
comment:1 Changed 3 years ago by
Authors:  → Jonathan Kliem 

Branch:  → public/29676 
Cc:  JeanPhilippe Labbé Laith Rastanawi added 
Commit:  → c6fe6d4578c25aaa3e465f8c74b1c500ff5d6946 
Status:  new → needs_review 
comment:2 Changed 3 years ago by
Could you extend a little bit in the description on what this does?
comment:3 Changed 3 years ago by
Description:  modified (diff) 

comment:4 Changed 3 years ago by
Milestone:  sage9.1 → sage9.2 

comment:5 Changed 2 years ago by
Cc:  Travis Scrimshaw added 

comment:6 Changed 2 years ago by
Shouldn't you also make the corresponding changes/additions to the pxd
file?
comment:7 Changed 2 years ago by
Branch:  public/29676 → public/29676reb 

Commit:  c6fe6d4578c25aaa3e465f8c74b1c500ff5d6946 → 81f43ea7f37111b5a920d51340cf0a65ba4dab4b 
comment:8 Changed 2 years ago by
Branch:  public/29676reb → public/29676reb2 

Commit:  81f43ea7f37111b5a920d51340cf0a65ba4dab4b 
Dependencies:  #28894 → #28894, #29654 
Merge conflict.
comment:9 Changed 2 years ago by
Commit:  → 81372b652d2dd92c87b80a0795bb276e49c67985 

Branch pushed to git repo; I updated commit sha1. New commits:
a6d2b2d  a face iterator subclass that yield PolyhedronFace

1476675  representation like Polyhedron_base

47fc7cd  account for FaceIterator > FaceIterator_base

295039a  documentation

d36da4a  coverage and small improvement

3f92d2d  fixed merge conflict

3ba34fd  move documenation to exposed class

109d60f  make nogil function of crucial methods in FaceIterator

ddd4687  expose nogil functions in header

81372b6  Merge branch 'develop' of git://trac.sagemath.org/sage into public/29676reb2

comment:10 Changed 2 years ago by
Reviewers:  → Travis Scrimshaw 

Status:  needs_review → positive_review 
LGTM.
comment:12 Changed 2 years ago by
Branch:  public/29676reb2 → 81372b652d2dd92c87b80a0795bb276e49c67985 

Resolution:  → fixed 
Status:  positive_review → closed 
Note: See
TracTickets for help on using
tickets.
New commits:
important attributes of iterator in structure
src/simplification of doctests
fixed failing doctest
make nogil function of crucial methods in FaceIterator