GeezerVison is showing github.com — Update to SuiteSparse:GraphBLAS 8.0.0 by eriknw · Pull Request #456 · python-graphblas/python-graphblas
Conversation
eriknw
commented
May 21, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
This begins updating to SuiteSparse:GraphBLAS 8.0.0, which added the JIT and contexts. Currently, this PR adds support for context and updates the configurations. We still need to support the JIT.
See release log here: https://github.com/DrTimothyAldenDavis/GraphBLAS/releases/tag/v8.0.0
Contexts
There are two types of contexts: global, and thread-local.
- The global context can be viewed and modified at
gb.global_context.
- Local contexts can be created by
gb.ss.Context().
- Local contexts can also be used as context managers, such as
with gb.ss.Context(nthreads=4):.
- Contexts also have
c.engage() and c.disengage() methods that are used by __enter__ and __exit__.
Local contexts do not stack. The inner context replaces the outer context here:
with Context(nthreads=8):
# nthreads is 8
with Context(nthreads=4):
# nthreads is 4
# Now nthreads is default (not 8!)I don't know if we want to try to keep track of the stack ourselves. Perhaps if/when somebody needs such a workflow.
The other tricky thing with contexts is that nthreads and chunks were removed as descriptor options. This is awkward for us, since I like the syntax of expr.new(nthreads=8) and C(nthreads=8) << expr and would like to keep it (and not add incompatible changes). So, I kept it. If any context option is used in **opts (which is typically for the descriptor), then we either update the current context that the user explicitly engaged, or we create a new context whose lifetime matches the descriptor lifetime. This ought to work well enough in practice, but I'm not fond of relying on __del__ to disengage the context. I would prefer if e.g. nthreads was still part of the descriptor.
Configs
Two changes happened with configuration in SuiteSparse:GraphBLAS:
nthreads and chunks were removed (they're now handled by context- many JIT options were added
We have some choices for how we can handle these.
First, the global config can have all configurations and global contexts:
>>> gb.ss.config
GlobalConfig({'bitmap_switch': [0.03999999910593033, 0.05000000074505806, 0.05999999865889549, 0.07999999821186066, 0.10000000149011612, 0.20000000298023224, 0.30000001192092896, 0.4000000059604645],
'burble': False,
'chunk': 65536.0,
'format': 'by_row',
'gpu_id': -1,
'hyper_switch': 0.0625,
'jit_c_cmake_libs': 'm;dl;/home/erik/miniconda3/envs/sspg/x86_64-conda-linux-gnu/lib/libgomp.so;/home/erik/miniconda3/envs/sspg/x86_64-conda-linux-gnu/sysroot/usr/lib/libpthread.so',
'jit_c_compiler_flags': ' -Wundef -std=c11 -lm -Wno-pragmas -fexcess-precision=fast -fcx-limited-range -fno-math-errno -fwrapv -O3 -DNDEBUG -fopenmp -fPIC ',
'jit_c_compiler_name': '/home/erik/miniconda3/envs/sspg/bin/cc',
'jit_c_control': 'on',
'jit_c_libraries': ' -lm -ldl /home/erik/miniconda3/envs/sspg/x86_64-conda-linux-gnu/lib/libgomp.so /home/erik/miniconda3/envs/sspg/x86_64-conda-linux-gnu/sysroot/usr/lib/libpthread.so',
'jit_c_linker_flags': ' -shared ',
'jit_c_preface': '',
'jit_cache_path': '/home/erik/.SuiteSparse/GrB8.0.0',
'jit_error_log': '',
'jit_use_cmake': False,
'memory_pool': [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
'nthreads': 8,
'print_1based': False})Second, we can have a new global_context object:
>>> gb.ss.global_context
GlobalContext({'chunk': 65536.0,
'gpu_id': -1,
'nthreads': 8})Third, we can have a new jit or jitconfig object:
>>> gb.ss.jit
JitConfig({'c_cmake_libs': 'm;dl;/home/erik/miniconda3/envs/sspg/x86_64-conda-linux-gnu/lib/libgomp.so;/home/erik/miniconda3/envs/sspg/x86_64-conda-linux-gnu/sysroot/usr/lib/libpthread.so',
'c_compiler_flags': ' -Wundef -std=c11 -lm -Wno-pragmas -fexcess-precision=fast -fcx-limited-range -fno-math-errno -fwrapv -O3 -DNDEBUG -fopenmp -fPIC ',
'c_compiler_name': '/home/erik/miniconda3/envs/sspg/bin/cc',
'c_control': 'on',
'c_libraries': ' -lm -ldl /home/erik/miniconda3/envs/sspg/x86_64-conda-linux-gnu/lib/libgomp.so /home/erik/miniconda3/envs/sspg/x86_64-conda-linux-gnu/sysroot/usr/lib/libpthread.so',
'c_linker_flags': ' -shared ',
'c_preface': '',
'cache_path': '/home/erik/.SuiteSparse/GrB8.0.0',
'error_log': '',
'use_cmake': False})So, if we want global_context and/or jit/jitconfig objects, we could remove one/both of them from the global config, which simplifies the global config considerably. For example, here's the global config without the JIT options:
GlobalConfig({'bitmap_switch': [0.03999999910593033, 0.05000000074505806, 0.05999999865889549, 0.07999999821186066, 0.10000000149011612, 0.20000000298023224, 0.30000001192092896, 0.4000000059604645],
'burble': False,
'chunk': 65536.0,
'format': 'by_row',
'gpu_id': -1,
'hyper_switch': 0.0625,
'memory_pool': [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
'nthreads': 8,
'print_1based': False})If we also removed the global context items from the global config, we get:
GlobalConfig({'bitmap_switch': [0.03999999910593033, 0.05000000074505806, 0.05999999865889549, 0.07999999821186066, 0.10000000149011612, 0.20000000298023224, 0.30000001192092896, 0.4000000059604645],
'burble': False,
'format': 'by_row',
'hyper_switch': 0.0625,
'memory_pool': [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
'print_1based': False})@jim22k @SultanOrazbayev how would you like to organize the global configuration and global context? It's helpful to expose the global context so users can do global_contect.engage() to reset the context. I also like keeping nthreads in the global config (which gets and sets via the global context). So, my main question I'm struggling with is whether to split out the jit config.
Finally, we don't yet support getting or setting functions such as malloc, free, print, and flush (the third change in 8.0.0).
JIT
Coming soon!
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
coveralls
commented
May 21, 2023
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
coverage: 99.44% (-0.4%) from 99.806%
when pulling 8ec612f on eriknw:v8.0.0
into f0e0324 on python-graphblas:main.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Alright, I think we should consider how we want contexts to behave and not rely on default behavior. For example, I think it would be nice to be able to layer contexts:
with Context(nthreads=4):
# nthreads is 4
with Context(chunk=0.5):
# nthreads is still 4 (new behavior!)
...
# nthreads is still 4 (new behavior!) |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I updated contexts to chain and stack by default as illustrated in my previous comment.
Descriptor-like context values (such as expr.new(nthreads=4)) create new context objects derived from the current active context, and they "disengage" when they go out of scope so they can now apply to GraphBLAS functions that don't have descriptors. I think everything should more-or-less behave exactly as expected/desired in all plausible usage (but it's probably possible to get contexts in a weird state if one tries, really, really hard to do so).
I/we still need to document and test descriptors, and work on the JIT. If anybody wants to continue to work or play with this PR, ping me on discord or slack.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
In response to this comment, DrTimothyAldenDavis/GraphBLAS#218 (comment), I wonder whether it would be better for us to have a single context per thread that we update (it sounds like engaging a context in a thread may be required for kernel fusion in the future, and engaging many contexts in a thread may wreak havoc on this). This would mean that Context objects that would get used on a regular basis are pure Python that don't create/free GxB_Context objects, but modify the thread-local context. I'm glad we learned of this now. I'd like to think on it a bit.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
JIT tests are now passing in CI for Linux and Windows! 🎉
|
SultanOrazbayev reacted with hooray emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
jit_ffi = FFI()
|
|
|
|
|
|
|
|
|
def register_new(name, jit_c_definition, *, np_type=None):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove the name argument here? It's required to be the same name as the typedef, and I bet we could determine the name from jit_c_definition.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
__call__ = TypedUserUnaryOp.__call__
|
|
|
|
|
|
|
|
|
def register_new(name, jit_c_definition, input_type, ret_type):
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, should we get the name, input type, and return type from the C definition here?
Alternatively, should name not be required to match the name used in the C definition?
Should we call this function something else, such as gb.unary.ss.register_jit?
Also, should we consider adding register_anonymous or allowing it to be anonymous?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
This is going in!
We still need to add some tests and documentaion, so SuiteSparse 8 is not yet "officially" supported. The metadata only allows SuiteSparse 7. This also gives us more time to review and refine the JIT.
Feel free to add review comments to this PR even after it's merged.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
btw, the JIT is working for CI for all OSes with both python-suitesparse-graphblas wheels and GraphBLAS from conda-forge except for Window wheels (I don't know why).
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|