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

Release GIL #25

Closed
sbarratt opened this issue Jun 29, 2019 · 12 comments
Closed

Release GIL #25

sbarratt opened this issue Jun 29, 2019 · 12 comments

Comments

@sbarratt
Copy link

sbarratt commented Jun 29, 2019 •
edited
Loading

Would it be possible to release the GIL before calling into osqp, with the macro Py_BEGIN_ALLOW_THREADS, or are there possible complications?

Similar to how this is done in scs.
@bstellato
Copy link
Contributor

bstellato commented Jun 30, 2019 •
edited
Loading

It should be very easy to implement since it is literally two lines of code per API function.

However, the OSQP interface is a bit more complicated than the SCS one since we divide the python commands exactly as in the C API with setup, solve, cleanup, see osqpobjectpy.h. In this way, we store the workspace internally and the OSQP Python object structure contains a pointer to the C workspace. I believe scs-python does not do so. I would probably disable GIL for all the Python functions corresponding to the OSQP C API functions.

It might be useful to write some unittests to make sure the internal OSQP memory does not get corrupted in multi-threading if we disable the GIL. Just a curiosity, for which application do you need to disable the GIL?

EDIT: After thinking about it, I am not 100% sure simply disabling the GIL would work since we are using the workspace structure which is shared between different functions calls.

@sbarratt
Copy link
Author

I think releasing the GIL would be most important for the solve function.

The application for disabling the GIL is solving multiple OSQP in parallel, in python, using threads. If the GIL is never released, then the calls will actually be concurrent. Using threads is preferred over processes because you don't have all the overhead of forking a new process, copying memory, etc...

@bstellato
Copy link
Contributor

The problem is that if we release the GIL, the workspace is shared between threads that will probably end up overwriting the ADMM iterates and other variables.

OSQP Python wrapper defines the workspace inside a python object and stores it so that we can reuse it in subsequent solves. SCS does not do that. Instead, it wraps the function scs which calls init, solve, finish all at once allocating and deallocating the whole the workspace.

One solution is to write a similar function for OSQP that executes the setup, solve and cleanup directly without creating an object. This would allocate the workspace, solve the problem and cleanup the allocation in one function call. Releasing the GIL would be easy and safe with it since all the variables, including the workspace, would be local to this new function.

However, if you called OSQP with many threads in this way, I suspect you would find similar bottlenecks as with multiprocessing. This is because when we run setup to create a new workspace, we perform all the initial solver steps (scaling, first factorization, etc.) including copying the problem data into memory. This would happen for all the threads in your program.

Let me know this solution would work for you anyway.

@sbarratt
Copy link
Author

The main motivation would be for here:
https://github.com/oxfordcontrol/osqpth/blob/master/osqpth/osqpth.py#L93
That is, where you are solving a bunch of (independent) OSQP problems, and you want to do it with multiple threads to take advantage of all your cores.

It seems like setup and solve are called together here.

@bstellato bstellato mentioned this issue Aug 15, 2019
Merged
@bstellato
Copy link
Contributor

bstellato commented Aug 16, 2019 •
edited
Loading

PR #29 should fix this issue. It adds a function osqp.solve that does not need any object. It performs setup solve and cleanup immediately and it disables the GIL. @sbarratt could you please try that branch?

In the osqpth repo you mentioned setup and solve are called together. However, I added some comments like this one where I believe it would be better to split setup and solve.

I am not sure what would be the most efficient way to do so (using setup + solve separately VS having a unique function that performs them together disabling the GIL).

@sbarratt
Copy link
Author

sbarratt commented Aug 19, 2019 via email

@bstellato
Copy link
Contributor

Unfortunately not. If you run setup once and then solve in parallel using threads with no GIL, they will change the workspace variables at the same time. This can create many troubles. The exact same issue applies to the SCS C library by the way.

@sbarratt
Copy link
Author

sbarratt commented Aug 19, 2019 via email

@sbarratt
Copy link
Author

sbarratt commented Aug 19, 2019 via email

@sbarratt
Copy link
Author

sbarratt commented Aug 24, 2019 via email

@sbarratt
Copy link
Author

sbarratt commented Sep 2, 2019

Still getting a seg fault. Here's the code I'm running:


import osqp
import numpy as np
from scipy import sparse

# Define problem data
P = sparse.csc_matrix([[4, 1], [1, 2]])
q = np.array([1, 1])
A = sparse.csc_matrix([[1, 1], [1, 0], [0, 1]])
l = np.array([1, 0, 0])
u = np.array([1, 0.7, 0.7])

osqp.solve(P, q, A, l, u)

@bstellato
Copy link
Contributor

There is an issue with printing enabled if that function is called. Let's continue in #33 to avoid duplicates.

Without printing this function should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bstellato @sbarratt