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.

Sign up for GitHub

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

Jump to bottom

Make a nogil version of the most important methods of FaceIterator. #29676

Closed
kliem opened this issue May 11, 2020 · 20 comments
Closed

Make a nogil version of the most important methods of FaceIterator. #29676

kliem opened this issue May 11, 2020 · 20 comments
Labels
c: geometry p: major / 3 t: enhancement
Milestone
sage-9.2

Comments

@kliem
Copy link
Contributor

kliem commented May 11, 2020

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.

Depends on #28894
Depends on #29654

CC: @jplab @LaisRast @tscrim

Component: geometry

Keywords: polytopes, f-vector

Author: Jonathan Kliem

Branch/Commit: 81372b6

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/29676
@kliem kliem added this to the sage-9.1 milestone May 11, 2020
@kliem
Copy link
Contributor Author

kliem commented May 11, 2020

Branch: public/29676

@kliem
Copy link
Contributor Author

kliem commented May 11, 2020

Author: Jonathan Kliem

@kliem
Copy link
Contributor Author

kliem commented May 11, 2020

Commit: c6fe6d4

@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
c6fe6d4make nogil function of crucial methods in FaceIterator

@jplab
Copy link

jplab commented May 12, 2020

comment:2

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

@kliem

This comment has been minimized.

Sign in to view
@mkoeppe mkoeppe modified the milestones: sage-9.1, sage-9.2 May 17, 2020
@tscrim
Copy link
Collaborator

tscrim commented Aug 11, 2020

comment:6

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

@kliem
Copy link
Contributor Author

kliem commented Aug 12, 2020

Changed commit from c6fe6d4 to 81f43ea

@kliem
Copy link
Contributor Author

kliem commented Aug 12, 2020

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

@kliem
Copy link
Contributor Author

kliem commented Aug 12, 2020

New commits:

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

@kliem
Copy link
Contributor Author

kliem commented Aug 12, 2020

Changed dependencies from #28894 to #28894, #29654

@kliem
Copy link
Contributor Author

kliem commented Aug 12, 2020

Changed commit from 81f43ea to none

@kliem
Copy link
Contributor Author

kliem commented Aug 12, 2020

comment:8

Merge conflict.

@kliem
Copy link
Contributor Author

kliem commented Aug 12, 2020

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 12, 2020

Commit: 81372b6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 12, 2020

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

@tscrim
Copy link
Collaborator

tscrim commented Aug 30, 2020

comment:10

LGTM.

@tscrim
Copy link
Collaborator

tscrim commented Aug 30, 2020

Reviewer: Travis Scrimshaw

@kliem
Copy link
Contributor Author

kliem commented Aug 30, 2020

comment:11

Thanks.

@vbraun
Copy link
Member

vbraun commented Sep 1, 2020

Changed branch from public/29676-reb2 to 81372b6

@vbraun vbraun closed this as completed in 0fbb133 Sep 1, 2020
This was referenced Aug 24, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: geometry p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-9.2
Development

No branches or pull requests
5 participants
@vbraun @tscrim @jplab @mkoeppe @kliem