JIT invocation from python for benchmarks #832

ohm314 · 2022-03-23T14:33:28Z

Created CodegenDriver class to factor out ast preparation
Created pybind wrappers for Jit and Codegen configuration options
misc small changes to simplify code

Here's a little test for this:

import sys
sys.path.pop(0)
print(sys.path)

import nmodl.dsl as nmodl

driver = nmodl.NmodlDriver()

cfg = nmodl.CodeGenConfig()
cfg.llvm_vector_width = 4
cfg.llvm_opt_level_ir = 2

with open('test/benchmark/kernels/hh.mod') as f:
    hh = f.read()
    ast = driver.parse_string(hh)
    jit = nmodl.Jit(cfg)

    res = jit.run(ast, "hh", 1000, 10000)
    print(res)

(updated example)

- Created CodegenDriver class to factor out ast preparation - Created pybind wrappers for Jit and Codegen configuration options - misc small changes to simplify code

bbpbuildbot · 2022-03-23T14:57:30Z

Logfiles from GitLab pipeline #43947 (:no_entry:) have been uploaded here!

Status and direct links:

iomaganaris

Overall LGTM. I just skimmed through the PR and left some comments/suggestions on its current state since it's still draft

src/codegen/codegen_driver.hpp

src/main.cpp

src/pybind/pynmodl.cpp

@iomaganaris

- Updated benchmark runner to return runtime stats - Updated python module to reflect this change and return results to python - Addressed @iomaganaris' comments.

bbpbuildbot · 2022-04-11T14:42:21Z

Logfiles from GitLab pipeline #47934 (:no_entry:) have been uploaded here!

Status and direct links:

bbpbuildbot · 2022-04-12T07:49:45Z

Logfiles from GitLab pipeline #48066 (:no_entry:) have been uploaded here!

Status and direct links:

bbpbuildbot · 2022-04-12T07:49:56Z

Logfiles from GitLab pipeline #48065 (:no_entry:) have been uploaded here!

Status and direct links:

iomaganaris

Some minor suggestions and comments.
Overall LGTM. I would just like to add an integration test that runs a small python example.
Also the python jit needs to have -DNMODL_PYTHON_BINDINGS_ENABLED to work but when I enable it with clang 13.0.0 I get issues like the following:

/usr/bin/ld: ../codegen/libcodegen.a(codegen_driver.cpp.o): relocation R_X86_64_32S against hidden symbol `_ZTVN5nmodl7visitor10AstVisitorE' can not be used when making a shared object
/usr/bin/ld: ../codegen/libcodegen.a(codegen_compatibility_visitor.cpp.o): relocation R_X86_64_32 against hidden symbol `_ZTIN5nmodl3ast3AstE' can not be used when making a shared object
/usr/bin/ld: ../../test/benchmark/libllvm_benchmark.a(llvm_benchmark.cpp.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: ../../test/benchmark/libllvm_benchmark.a(jit_driver.cpp.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: ../../test/unit/libbenchmark_data.a(codegen_data_helper.cpp.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC

Compiling NMODL with -DCMAKE_CXX_FLAGS="-fPIC" fixes the compilation but then I get the following error when I run the example provided:

➜  build_pyjit git:(ohm314/pyjit) python test_pyjit.py                                                                                                                                                                                                 
['/home/magkanar/bbp_repos/nmodl/build_pyjit', '/home/magkanar/bbp_repos/nmodl/build_pyjit/install/lib/python', '/home/magkanar/bbp_repos/nmodl/build_pyjit/install/lib#', '/home/magkanar/bbp_repos/nmodl/build_pyjit/install/lib', '/usr/lib/python38.zip', '/usr/lib/python3.8', '/usr/lib/python3.8/lib-dynload', '/home/magkanar/.local/lib/python3.8/site-packages', '/usr/local/lib/python3.8/dist-packages', '/usr/lib/python3/dist-packages']
[NMODL] [warning] :: Python bindings are not available
Traceback (most recent call last):
  File "test_pyjit.py", line 5, in <module>
    import nmodl.dsl as nmodl
  File "/home/magkanar/bbp_repos/nmodl/build_pyjit/install/lib/nmodl/dsl.py", line 4, in <module>
    from ._nmodl import *
ImportError: /home/magkanar/bbp_repos/nmodl/build_pyjit/install/lib/nmodl/_nmodl.cpython-38-x86_64-linux-gnu.so: undefined symbol: _ZTVN5nmodl7codegen20CodegenHelperVisitorE

Am I missing something? How did you build/test this?

test/benchmark/llvm_benchmark.cpp

src/main.cpp

bbpbuildbot · 2022-04-12T13:09:42Z

Logfiles from GitLab pipeline #48181 (:no_entry:) have been uploaded here!

Status and direct links:

- Move fmt::literals namespace using into the nmodl namespace (where it should be). - Remove also logger.hpp header inclusion from places where it is unnecessary.

bbpbuildbot · 2022-04-21T09:42:27Z

Logfiles from GitLab pipeline #49638 (:no_entry:) have been uploaded here!

Status and direct links:

src/codegen/codegen_driver.cpp

Passing the dereferenced node and constructing new shared_ptrs when needed causes all kinds of grief.

bbpbuildbot · 2022-04-22T12:31:59Z

Logfiles from GitLab pipeline #49989 (:no_entry:) have been uploaded here!

Status and direct links:

bbpbuildbot · 2022-04-25T14:46:27Z

Logfiles from GitLab pipeline #50460 (:white_check_mark:) have been uploaded here!

Status and direct links:

pramodk

really nice! was able to run this locally!

$ PYTHONPATH=`pwd`/lib/ python3 test.py
...
[NMODL] [info] :: Created LLVM IR module from NMODL AST in 1.9729e-05 sec
[NMODL] [info] :: CPU: icelake-client
[NMODL] [info] :: Benchmarking kernel 'nrn_state_hh' with 2.708770751953125 MBs dataset
[NMODL] [info] :: Average compute time = 0.000038
[NMODL] [info] :: Compute time standard deviation = 0.000000
[NMODL] [info] :: Minimum compute time = 0.000025
[NMODL] [info] :: Maximum compute time = 0.003346

{'nrn_state_hh': (3.821874700000001e-05, 1.229097320882649e-07, 2.4908e-05, 0.003346102)}

All looks good to me! If you ask me, I would have two questions/wishes:

as python bindings are required for running all meaningful tests, I wonder if we should turn ON bindings now (we turned off in the llvm branch during initial development):

 option(NMODL_ENABLE_PYTHON_BINDINGS "Enable pybind11 based python bindings" OFF)

to avoid this getting broken, it will be helpful to add a minimal test (like the one shown in the description) here.

src/pybind/CMakeLists.txt

src/main.cpp

src/codegen/llvm/target_platform.hpp

src/codegen/codegen_driver.hpp

iomaganaris

LGTM apart from the existing comments.
Regarding enabling NMODL_ENABLE_PYTHON_BINDINGS I think we can still keep it off by default to accelerate local development when the python bindings are not needed however we can enable them on the CI.
I'm also in favour of adding a test that uses the python JIT.
In https://github.com/BlueBrain/nmodl/pull/829/files#diff-037ea159eb0a7cb0ac23b851e66bee30fb838ee8d0d99fa331a1ba65283d37f7R93 I've added tests for running the JIT on the gitlab ci in allocations on BB5 so we can use the same configuration here to run the tests

ohm314 · 2022-04-26T13:29:02Z

I will add a test. @iomaganaris already suggested it too and you are absolutely right.

bbpbuildbot · 2022-04-27T10:51:55Z

Logfiles from GitLab pipeline #51014 (:white_check_mark:) have been uploaded here!

Status and direct links:

iomaganaris

LGTM

@iomaganaris

- Created CodegenDriver class to factor out ast preparation - Created pybind wrappers for Jit and Codegen configuration options - Updated benchmark runner to return runtime stats - Return benchmark results to python - Addressed @iomaganaris' comments. - Add a PyJIT integration test * Re-enable python bindings by default

@iomaganaris

- Created CodegenDriver class to factor out ast preparation - Created pybind wrappers for Jit and Codegen configuration options - Updated benchmark runner to return runtime stats - Return benchmark results to python - Addressed @iomaganaris' comments. - Add a PyJIT integration test * Re-enable python bindings by default

@iomaganaris

- Created CodegenDriver class to factor out ast preparation - Created pybind wrappers for Jit and Codegen configuration options - Updated benchmark runner to return runtime stats - Return benchmark results to python - Addressed @iomaganaris' comments. - Add a PyJIT integration test * Re-enable python bindings by default

@iomaganaris

- Created CodegenDriver class to factor out ast preparation - Created pybind wrappers for Jit and Codegen configuration options - Updated benchmark runner to return runtime stats - Return benchmark results to python - Addressed @iomaganaris' comments. - Add a PyJIT integration test * Re-enable python bindings by default

Proof of Concept jit invocation from python

0f9fbb6

- Created CodegenDriver class to factor out ast preparation - Created pybind wrappers for Jit and Codegen configuration options - misc small changes to simplify code

iomaganaris reviewed Mar 24, 2022

View reviewed changes

src/codegen/codegen_driver.hpp Outdated Show resolved Hide resolved

src/main.cpp Show resolved Hide resolved

src/main.cpp Show resolved Hide resolved

src/main.cpp Show resolved Hide resolved

src/pybind/pynmodl.cpp Outdated Show resolved Hide resolved

Return benchmark results from runner

d3af30b

- Updated benchmark runner to return runtime stats - Updated python module to reflect this change and return results to python - Addressed @iomaganaris' comments.

Provide Jit constructor that takes configuration object

00a8c95

iomaganaris reviewed Apr 12, 2022

View reviewed changes

test/benchmark/llvm_benchmark.cpp Outdated Show resolved Hide resolved

src/main.cpp Show resolved Hide resolved

ohm314 force-pushed the ohm314/pyjit branch from 00a8c95 to 78ed565 Compare April 12, 2022 12:52

ohm314 marked this pull request as ready for review April 12, 2022 19:48

Omar Awile added 4 commits April 20, 2022 15:30

Remove unnecessary headers

c9cd03f

Merge branch 'llvm' into ohm314/pyjit

7607890

Changes that belonged to the merge

e13adb3

Fix compile issue with pybind11 literals

ff1fc14

- Move fmt::literals namespace using into the nmodl namespace (where it should be). - Remove also logger.hpp header inclusion from places where it is unnecessary.

ohm314 force-pushed the ohm314/pyjit branch from 78ed565 to ff1fc14 Compare April 21, 2022 09:20

iomaganaris reviewed Apr 21, 2022

View reviewed changes

src/codegen/codegen_driver.cpp Outdated Show resolved Hide resolved

Pass the shared_ptr to Program along to CodegenDriver

0f34eec

Passing the dereferenced node and constructing new shared_ptrs when needed causes all kinds of grief.

Compile targets that are linked to python module with -fPIC

923cb65

ohm314 requested review from iomaganaris and pramodk April 25, 2022 14:50

pramodk reviewed Apr 25, 2022

View reviewed changes

src/pybind/CMakeLists.txt Outdated Show resolved Hide resolved

src/main.cpp Show resolved Hide resolved

src/codegen/llvm/target_platform.hpp Show resolved Hide resolved

src/codegen/codegen_driver.hpp Outdated Show resolved Hide resolved

iomaganaris reviewed Apr 26, 2022

View reviewed changes

Omar Awile added 2 commits April 27, 2022 11:52

Add a PyJIT integration test

4c86ae4

Formatting

b3983d8

Cleanup CMakeLists.txt

a38f54a

ohm314 changed the title ~~Proof of Concept jit invocation from python~~ JIT invocation from python for benchmarks Apr 27, 2022

Enable python bindings by default

40b8c9d

iomaganaris approved these changes Apr 27, 2022

View reviewed changes

ohm314 merged commit c154efb into llvm Apr 27, 2022

ohm314 deleted the ohm314/pyjit branch April 27, 2022 12:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT invocation from python for benchmarks #832

JIT invocation from python for benchmarks #832

ohm314 commented Mar 23, 2022 •

edited

Loading

bbpbuildbot commented Mar 23, 2022

iomaganaris left a comment

bbpbuildbot commented Apr 11, 2022

bbpbuildbot commented Apr 12, 2022

bbpbuildbot commented Apr 12, 2022

iomaganaris left a comment

bbpbuildbot commented Apr 12, 2022

bbpbuildbot commented Apr 21, 2022

bbpbuildbot commented Apr 22, 2022

bbpbuildbot commented Apr 25, 2022

pramodk left a comment

iomaganaris left a comment

ohm314 commented Apr 26, 2022

bbpbuildbot commented Apr 27, 2022

iomaganaris left a comment

JIT invocation from python for benchmarks #832

JIT invocation from python for benchmarks #832

Conversation

ohm314 commented Mar 23, 2022 • edited Loading

bbpbuildbot commented Mar 23, 2022

iomaganaris left a comment

Choose a reason for hiding this comment

bbpbuildbot commented Apr 11, 2022

bbpbuildbot commented Apr 12, 2022

bbpbuildbot commented Apr 12, 2022

iomaganaris left a comment

Choose a reason for hiding this comment

bbpbuildbot commented Apr 12, 2022

bbpbuildbot commented Apr 21, 2022

bbpbuildbot commented Apr 22, 2022

bbpbuildbot commented Apr 25, 2022

pramodk left a comment

Choose a reason for hiding this comment

iomaganaris left a comment

Choose a reason for hiding this comment

ohm314 commented Apr 26, 2022

bbpbuildbot commented Apr 27, 2022

iomaganaris left a comment

Choose a reason for hiding this comment

ohm314 commented Mar 23, 2022 •

edited

Loading