Nanobind #99

iglesias · 2024-05-17T13:33:28Z

I thought next to be adding to this PR (1) a workflow to have the Python interface in the CI and (2) more of the parameters in nanobind_extension.cpp. I should also try using other of the DR methods from Python and consider if it could be nice (and easy) to have different combinations covered in the CI too.

And well also the bunch of TODO's I left ':-D

So, hmm, there could be a lot. I think I will continue pushing step by step and keep an eye how it can start getting integrated.

>>> import lib.tapkee Traceback (most recent call last): File "<stdin>", line 1, in <module> ImportError: /home/garcia/tapkee/tapkee/lib/tapkee.cpython-312-x86_64-linux-gnu.so: undefined symbol: dsaupd_ And go a bit too far with the renaming.

message.

build_nanobind.sh

lisitsyn

That's an amazing thing there. It looks much simpler than I thought.

CMakeLists.txt

lisitsyn · 2024-05-18T12:20:11Z

examples/nanobind.py

+    test_exception_unknown_method()
+    parameters = tapkee.ParametersSet()
+    method = parse_reduction_method('lle')
+    parameters.add(tapkee.Parameter.create('dimension reduction method', method))


Could you add some numeric parameter too?

There's one for target_dimension now used from test_nanobind_extension.py.

iglesias · 2024-05-20T05:16:55Z

test/test_nanobind_extension.py

Refactor to use e.g. pytest.

iglesias · 2024-05-20T05:18:24Z

Running in the CI looking good too!
https://github.com/iglesias/tapkee/actions/runs/9153626706/job/25162869366

I will update the workflow since it would be nice if the new job appears in the pull request too.

iglesias · 2024-05-21T04:44:18Z

We saw that it worked with nanobind and now know what to expect from it. It can be picked up at any moment when there's interest in having it with yesterday's new parse_* methods and templates.

iglesias · 2024-06-04T10:35:06Z

I quickly merged the upstream changes in examples/go.py. Next, I recall from two weeks ago there had also been changes in the templates, so I expect the nanobind build to fail.

Testing pending.

iglesias · 2024-06-04T13:54:20Z

I started updating the extension and got to this one:

src/python/nanobind_extension.cpp: In function ‘void nanobind_init_tapkee(nanobind::module_&)’:
src/python/nanobind_extension.cpp:38:10: error: no matching function for call to ‘nanobind::module_::def(const char [15], <unresolved overloaded function type>)’
   38 |     m.def("parse_multiple", &parse_multiple<DimensionReductionMethod>);
      |     ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nanobind.h:51:venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nb_func.h:255:10: note: candidate: ‘template<class Func, class ... Extra> nanobind::module_& nanobind::module_::def(const char*, Func&&, const Extra& ...)’
  255 | module_ &module_::def(const char *name_, Func &&f, const Extra &...extra) {
      |          ^~~~~~~
venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nb_func.h:255:10: note:   template argument deduction/substitution failed:
src/python/nanobind_extension.cpp:38:10: note:   couldn’t deduce template parameter ‘Func’
   38 |     m.def("parse_multiple", &parse_multiple<DimensionReductionMethod>)

iglesias · 2024-06-14T10:15:26Z

I started updating the extension and got to this one:

src/python/nanobind_extension.cpp: In function ‘void nanobind_init_tapkee(nanobind::module_&)’:
src/python/nanobind_extension.cpp:38:10: error: no matching function for call to ‘nanobind::module_::def(const char [15], <unresolved overloaded function type>)’
   38 |     m.def("parse_multiple", &parse_multiple<DimensionReductionMethod>);
      |     ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nanobind.h:51:venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nb_func.h:255:10: note: candidate: ‘template<class Func, class ... Extra> nanobind::module_& nanobind::module_::def(const char*, Func&&, const Extra& ...)’
  255 | module_ &module_::def(const char *name_, Func &&f, const Extra &...extra) {
      |          ^~~~~~~
venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nb_func.h:255:10: note:   template argument deduction/substitution failed:
src/python/nanobind_extension.cpp:38:10: note:   couldn’t deduce template parameter ‘Func’
   38 |     m.def("parse_multiple", &parse_multiple<DimensionReductionMethod>)

I am still working on other build errors after this one. For this one the DimensionReductionMethod is not the right template, it'd be a map.

lisitsyn · 2024-06-14T11:31:07Z

I started updating the extension and got to this one:

src/python/nanobind_extension.cpp: In function ‘void nanobind_init_tapkee(nanobind::module_&)’:
src/python/nanobind_extension.cpp:38:10: error: no matching function for call to ‘nanobind::module_::def(const char [15], <unresolved overloaded function type>)’
   38 |     m.def("parse_multiple", &parse_multiple<DimensionReductionMethod>);
      |     ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nanobind.h:51:venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nb_func.h:255:10: note: candidate: ‘template<class Func, class ... Extra> nanobind::module_& nanobind::module_::def(const char*, Func&&, const Extra& ...)’
  255 | module_ &module_::def(const char *name_, Func &&f, const Extra &...extra) {
      |          ^~~~~~~
venv-tapkee-nanobind/lib/python3.12/site-packages/nanobind/include/nanobind/nb_func.h:255:10: note:   template argument deduction/substitution failed:
src/python/nanobind_extension.cpp:38:10: note:   couldn’t deduce template parameter ‘Func’
   38 |     m.def("parse_multiple", &parse_multiple<DimensionReductionMethod>)

I am still working on other build errors after this one. For this one the DimensionReductionMethod is not the right template, it'd be a map.

I'd suggest to introduce some currying so that there is an additional parse_method that just fixes the first argument of parse_multiple. This way you won't have to call a templated function. What do you think?

iglesias · 2024-06-14T18:23:26Z

I'd suggest to introduce some currying so that there is an additional parse_method that just fixes the first argument of parse_multiple. This way you won't have to call a templated function. What do you think?

Yeah! I think that's a good idea. It'd also simplify the extension in avoiding to expose the maps.

iglesias · 2024-06-16T11:05:44Z

The extension is back: https://github.com/iglesias/tapkee/actions/runs/9535436521/job/26281156242

There was a funny interaction with the renaming from withParameters to with since, I guess, it's a reserved word in Python.

For next, it appears the branch cannot be rebased due to conflicts. I haven't looked into it yet.

iglesias · 2024-06-17T14:34:04Z

Ah, all right, I think I understand about the rebase now. It can be squashed at least, I think that's suitable for this one.
I think it can be considered in a state to be ready for merge, then, it enables using the chain interface nicely from Python as well as directly inputting np.arrays to embed 🎉

lisitsyn · 2024-06-17T16:32:01Z

Do you think we squash and merge or would you force-squash it?

iglesias · 2024-06-17T18:45:24Z

I can do it here right away.

iglesias added 9 commits May 10, 2024 00:12

First steps interfacing Python with nanobind.

570aac6

Add parse_reduction_method and refactor Python to separate file.

dab05f8

Finish PoC.

c129206

Use dataset.

f0d43ea

Improve extension runner with plot.

461be6d

WIP start cmake for nanobind based on pip installation. Didn't work.

6b2b9bc

Link with required libraries. Fixes the error in the previous commit

881c800

message.

Update CMakeLists.txt with TODOs.

5a1e35d

iglesias requested a review from lisitsyn May 17, 2024 13:33

iglesias commented May 17, 2024

View reviewed changes

build_nanobind.sh Outdated Show resolved Hide resolved

lisitsyn reviewed May 18, 2024

View reviewed changes

iglesias added 3 commits May 19, 2024 18:21

Add CI workflow (build nb ext., still misses run/test from Python).

1f18383

Integrate basic test.

07910db

Install numpy in CI job. Remove build script substituted by CMake.

814bf78

iglesias commented May 20, 2024

View reviewed changes

test/test_nanobind_extension.py Outdated

Copy link

Collaborator Author

iglesias May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor to use e.g. pytest.

iglesias marked this pull request as ready for review May 20, 2024 05:18

iglesias closed this May 21, 2024

iglesias reopened this Jun 4, 2024

Merge remote-tracking branch 'master/main' into nanobind

a73d276

Testing pending.

iglesias force-pushed the nanobind branch from 3efbd16 to 127968b Compare June 16, 2024 10:56

Update nanobind extension.

127968b

iglesias merged commit 04b33b5 into lisitsyn:main Jun 17, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nanobind #99

Nanobind #99

iglesias commented May 17, 2024

lisitsyn left a comment

lisitsyn May 18, 2024

iglesias May 20, 2024

iglesias May 20, 2024

iglesias commented May 20, 2024

iglesias commented May 21, 2024

iglesias commented Jun 4, 2024

iglesias commented Jun 4, 2024 •

edited

Loading

iglesias commented Jun 14, 2024

lisitsyn commented Jun 14, 2024

iglesias commented Jun 14, 2024

iglesias commented Jun 16, 2024

iglesias commented Jun 17, 2024

lisitsyn commented Jun 17, 2024

iglesias commented Jun 17, 2024

Nanobind #99

Nanobind #99

Conversation

iglesias commented May 17, 2024

lisitsyn left a comment

Choose a reason for hiding this comment

lisitsyn May 18, 2024

Choose a reason for hiding this comment

iglesias May 20, 2024

Choose a reason for hiding this comment

iglesias May 20, 2024

Choose a reason for hiding this comment

iglesias commented May 20, 2024

iglesias commented May 21, 2024

iglesias commented Jun 4, 2024

iglesias commented Jun 4, 2024 • edited Loading

iglesias commented Jun 14, 2024

lisitsyn commented Jun 14, 2024

iglesias commented Jun 14, 2024

iglesias commented Jun 16, 2024

iglesias commented Jun 17, 2024

lisitsyn commented Jun 17, 2024

iglesias commented Jun 17, 2024

iglesias commented Jun 4, 2024 •

edited

Loading