#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: jipilab, gh-LaisRast, tscrim 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 18 months ago by gh-kliem

  • Authors set to Jonathan Kliem
  • Branch set to public/29676
  • Cc jipilab gh-LaisRast added
  • Commit set to c6fe6d4578c25aaa3e465f8c74b1c500ff5d6946
  • Status changed from new to needs_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 18 months ago by jipilab

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

comment:3 Changed 18 months ago by gh-kliem

  • Description modified (diff)

comment:4 Changed 18 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:5 Changed 15 months ago by mkoeppe

  • Cc tscrim added

comment:6 Changed 15 months ago by tscrim

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

comment:7 Changed 15 months ago by gh-kliem

  • Branch changed from public/29676 to public/29676-reb
  • Commit changed from c6fe6d4578c25aaa3e465f8c74b1c500ff5d6946 to 81f43ea7f37111b5a920d51340cf0a65ba4dab4b

New commits:

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

comment:8 Changed 15 months ago by gh-kliem

  • Branch changed from public/29676-reb to public/29676-reb2
  • Commit 81f43ea7f37111b5a920d51340cf0a65ba4dab4b deleted
  • Dependencies changed from #28894 to #28894, #29654

Merge conflict.

comment:9 Changed 15 months ago by git

  • Commit set to 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 14 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:11 Changed 14 months ago by gh-kliem

Thanks.

comment:12 Changed 14 months ago by vbraun

  • Branch changed from public/29676-reb2 to 81372b652d2dd92c87b80a0795bb276e49c67985
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.