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: gh-kliem Owned by:
Priority: major Milestone: sage-9.2
Component: geometry Keywords: polytopes, f-vector
Cc: Jean-Philippe 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:

Status badges

Description (last modified by gh-kliem)

We outsource 3 crucial methods of FaceIterator to nogil functions.

This is part of #28893: Parallel f-vector 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 gh-kliem

Authors: Jonathan Kliem
Branch: public/29676
Cc: Jean-Philippe Labbé Laith Rastanawi added
Commit: c6fe6d4578c25aaa3e465f8c74b1c500ff5d6946
Status: newneeds_review

New commits:

0c44221important attributes of iterator in structure
efb0bd3src/simplification of doctests
53fd2a2fixed failing doctest
c6fe6d4make nogil function of crucial methods in FaceIterator

comment:2 Changed 3 years ago by Jean-Philippe Labbé

Could you extend a little bit in the description on what this does?

comment:3 Changed 3 years ago by gh-kliem

Description: modified (diff)

comment:4 Changed 3 years ago by Matthias Köppe

Milestone: sage-9.1sage-9.2

comment:5 Changed 2 years ago by Matthias Köppe

Cc: Travis Scrimshaw added

comment:6 Changed 2 years ago by Travis Scrimshaw

Shouldn't you also make the corresponding changes/additions to the pxd file?

comment:7 Changed 2 years ago by gh-kliem

Branch: public/29676public/29676-reb
Commit: c6fe6d4578c25aaa3e465f8c74b1c500ff5d694681f43ea7f37111b5a920d51340cf0a65ba4dab4b

New commits:

9045767make nogil function of crucial methods in FaceIterator
81f43eaexpose nogil functions in header

comment:8 Changed 2 years ago by gh-kliem

Branch: public/29676-rebpublic/29676-reb2
Commit: 81f43ea7f37111b5a920d51340cf0a65ba4dab4b
Dependencies: #28894#28894, #29654

Merge conflict.

comment:9 Changed 2 years ago by git

Commit: 81372b652d2dd92c87b80a0795bb276e49c67985

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

a6d2b2da face iterator subclass that yield PolyhedronFace
1476675representation like Polyhedron_base
47fc7cdaccount for FaceIterator -> FaceIterator_base
295039adocumentation
d36da4acoverage and small improvement
3f92d2dfixed merge conflict
3ba34fdmove documenation to exposed class
109d60fmake nogil function of crucial methods in FaceIterator
ddd4687expose nogil functions in header
81372b6Merge branch 'develop' of git://trac.sagemath.org/sage into public/29676-reb2

comment:10 Changed 2 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

LGTM.

comment:11 Changed 2 years ago by gh-kliem

Thanks.

comment:12 Changed 2 years ago by Volker Braun

Branch: public/29676-reb281372b652d2dd92c87b80a0795bb276e49c67985
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.