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.

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

Switch to nanobind #107

Merged
merged 6 commits into from
Sep 11, 2023
Merged

Switch to nanobind #107

merged 6 commits into from
Sep 11, 2023

Conversation

matthiasdiener
Copy link
Contributor

@matthiasdiener matthiasdiener commented May 30, 2023

To-do:

  • Bump version to 2023.2
  • Leaks (as pointed out by nanobind at interpreter exit)
  • Fix docs
  • Apple build
  • Apple wheel build
  • Support build with barvinok
  • Get loopy to pass
  • Clean up commit history

@matthiasdiener matthiasdiener changed the title Nanobind Switch to nanobind May 30, 2023
@matthiasdiener matthiasdiener force-pushed the nanobind branch 2 times, most recently from ce0596b to ac0a047 Compare May 30, 2023 01:24
@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented May 30, 2023

I'm not sure what to do about this type of error, which is currently blocking this from getting compiled:

  In file included from /Users/mdiener/Work/emirge/islpy/src/wrapper/wrap_isl_part1.cpp:1:
  In file included from /Users/mdiener/Work/emirge/islpy/src/wrapper/wrap_isl.hpp:1:
  In file included from /Users/mdiener/Work/emirge/islpy/src/wrapper/wrap_helpers.hpp:4:
  In file included from /private/var/folders/w4/9hgdmgy56394v5rv6hxx2yk80000gn/T/pip-build-env-wctoly79/overlay/lib/python3.11/site-packages/nanobind/include/nanobind/nanobind.h:51:
  /private/var/folders/w4/9hgdmgy56394v5rv6hxx2yk80000gn/T/pip-build-env-wctoly79/overlay/lib/python3.11/site-packages/nanobind/include/nanobind/nb_func.h:129:34: error: cannot initialize a parameter of type 'char *' with an rvalue of type 'const char *'
                         cap->func(((make_caster<Args> &&) in.template get<Is>())
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /private/var/folders/w4/9hgdmgy56394v5rv6hxx2yk80000gn/T/pip-build-env-wctoly79/overlay/lib/python3.11/site-packages/nanobind/include/nanobind/nb_func.h:161:13: note: in instantiation of function template specialization 'nanobind::detail::func_create<false, true, int (*&)(const isl::multi_val &, isl_dim_type, char *), int, const isl::multi_val &, isl_dim_type, char *, 0UL, 1UL, 2UL, nanobind::scope, nanobind::name, nanobind::is_method, nanobind::arg, nanobind::arg, char[233]>' requested here
      detail::func_create<false, true>(
              ^
  /private/var/folders/w4/9hgdmgy56394v5rv6hxx2yk80000gn/T/pip-build-env-wctoly79/overlay/lib/python3.11/site-packages/nanobind/include/nanobind/nb_class.h:323:9: note: in instantiation of function template specialization 'nanobind::cpp_function_def<int, const isl::multi_val &, isl_dim_type, char *, nanobind::scope, nanobind::name, nanobind::is_method, nanobind::arg, nanobind::arg, char[233]>' requested here
          cpp_function_def((detail::forward_t<Func>) f, scope(*this), name(name_),
          ^
  /Users/mdiener/Work/emirge/islpy/src/wrapper/gen-expose-part1.inc:696:16: note: in instantiation of function template specialization 'nanobind::class_<isl::multi_val>::def<int (&)(const isl::multi_val &, isl_dim_type, char *), nanobind::arg, nanobind::arg, char[233]>' requested here
  wrap_multi_val.def("find_dim_by_name", isl::multi_val_find_dim_by_name, py::arg("type"), py::arg("name"), "find_dim_by_name(self, type, name)\n\n:param self: :class:`MultiVal`\n:param type: :class:`dim_type`\n:param name: string\n:return: int \n\n.. warning::\n\n    This function is not part of the officially public isl API. Use at your own risk.");
                 ^

A potential reason may be https://nanobind.readthedocs.io/en/latest/faq.html#how-can-i-preserve-the-const-ness-of-values-in-bindings

Edit:
#108 resolves this, and compilation succeeds with that fix.

@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented May 31, 2023

The current version compiles but crashes early during execution with:

$ python examples/demo.py
Traceback (most recent call last):
  File "/Users/mdiener/Work/emirge/islpy/examples/demo.py", line 6, in <module>
    .add_constraint(isl.Constraint.ineq_from_names(space, {1: -1, "x": 1}))
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mdiener/Work/emirge/miniforge3/envs/ceesd/lib/python3.11/site-packages/islpy/__init__.py", line 589, in ineq_from_names
    return c.set_coefficients_by_name(coefficients)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mdiener/Work/emirge/miniforge3/envs/ceesd/lib/python3.11/site-packages/islpy/__init__.py", line 486, in obj_set_coefficients_by_name
    name_to_dim = self.get_space().get_var_dict()
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mdiener/Work/emirge/miniforge3/envs/ceesd/lib/python3.11/site-packages/islpy/__init__.py", line 929, in wrapper
    return basic_method(basic_instance, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mdiener/Work/emirge/miniforge3/envs/ceesd/lib/python3.11/site-packages/islpy/__init__.py", line 403, in space_get_var_dict
    name = self.get_dim_name(tp, i)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mdiener/Work/emirge/miniforge3/envs/ceesd/lib/python3.11/site-packages/islpy/__init__.py", line 929, in wrapper
    return basic_method(basic_instance, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: nanobind::cast_error
nanobind: leaked 74 instances!
nanobind: leaked 84 types!
 - leaked type "islpy._isl.SetList"
 - leaked type "islpy._isl.AstNode"
 - leaked type "islpy._isl.MultiAff"
 - leaked type "islpy._isl.AccessInfo"
 - leaked type "islpy._isl.ScheduleNode"
 - leaked type "islpy._isl.MapList"
 - leaked type "islpy._isl.UnionPwMultiAffList"
 - leaked type "islpy._isl.ast_loop_type"
 - leaked type "islpy._isl.QPolynomial"
 - leaked type "islpy._isl.PwMultiAff"
 - leaked type "islpy._isl.QPolynomialFold"
 - ... skipped remainder
nanobind: leaked 3151 functions!
 - leaked function "to_str"
 - leaked function "flatten"
 - leaked function "union"
 - leaked function "sub"
 - leaked function "empty_space"
 - leaked function "get_list"
 - leaked function "set_max_operations"
 - leaked function "is_bijective"
 - leaked function "_is_valid"
 - leaked function "from_pw_aff"
 - leaked function "remove_dims"
 - ... skipped remainder
nanobind: this is likely caused by a reference counting issue in the binding code.

Edit:

Crash fixed by 4b8c175.

@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented Jun 2, 2023

I've traced the crash back to

#0  __cxxabiv1::__cxa_throw (obj=0x555555b9b090, tinfo=0x7ffff6e52318 <typeinfo for std::bad_cast>, dest=0x7ffff6d5d1b2 <std::bad_cast::~bad_cast()>)
    at ../../../../libstdc++-v3/libsupc++/eh_throw.cc:80
#1  0x00007ffff7132283 in nanobind::detail::raise_cast_error () at /tmp/pip-build-env-wwqep4xn/overlay/lib/python3.11/site-packages/nanobind/src/common.cpp:89
#2  0x00007ffff71877c3 in nanobind::cast<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > (value=...,
    policy=policy@entry=nanobind::rv_policy::automatic_reference) at /tmp/pip-build-env-wwqep4xn/overlay/lib/python3.11/site-packages/nanobind/include/nanobind/nb_cast.h:388
#3  0x00007ffff718a074 in isl::space_get_dim_name (arg_self=..., arg_type=arg_type@entry=isl_dim_out, arg_pos=arg_pos@entry=0)
    at /shared/home/mdiener/Work/emirge/islpy/src/wrapper/gen-wrap-part1.inc:88702
#4  0x00007ffff7160293 in nanobind::detail::func_create<false, true, nanobind::object (*&)(isl::space const&, isl_dim_type, unsigned int), nanobind::object, isl::space const&, isl_dim_type, unsigned int, 0ul, 1ul, 2ul, nanobind::scope, nanobind::name, nanobind::is_method, nanobind::arg, nanobind::arg, char [237]>(nanobind::object (*&)(isl::space const&, isl_dim_type, unsigned int), nanobind::object (*)(isl::space const&, isl_dim_type, unsigned int), std::integer_sequence<unsigned long, 0ul, 1ul, 2ul>, nanobind::scope const&, nanobind::name const&, nanobind::is_method const&, nanobind::arg const&, nanobind::arg const&, char const (&) [237])::{lambda(void*, _object**, unsigned char*, nanobind::rv_policy, nanobind::detail::cleanup_list*)#1}::operator()(void*, _object**, unsigned char*, nanobind::rv_policy, nanobind::detail::cleanup_list*) const (__closure=0x0, policy=<optimized out>,
    cleanup=<optimized out>, args_flags=<optimized out>, args=<optimized out>, p=<optimized out>)
    at /tmp/pip-build-env-wwqep4xn/overlay/lib/python3.11/site-packages/nanobind/include/nanobind/nb_func.h:128
#5  nanobind::detail::func_create<false, true, nanobind::object (*&)(isl::space const&, isl_dim_type, unsigned int), nanobind::object, isl::space const&, isl_dim_type, unsigned int, 0ul, 1ul, 2ul, nanobind::scope, nanobind::name, nanobind::is_method, nanobind::arg, nanobind::arg, char [237]>(nanobind::object (*&)(isl::space const&, isl_dim_type, unsigned int), nanobind::object (*)(isl::space const&, isl_dim_type, unsigned int), std::integer_sequence<unsigned long, 0ul, 1ul, 2ul>, nanobind::scope const&, nanobind::name const&, nanobind::is_method const&, nanobind::arg const&, nanobind::arg const&, char const (&) [237])::{lambda(void*, _object**, unsigned char*, nanobind::rv_policy, nanobind::detail::cleanup_list*)#1}::_FUN(void*, _object**, unsigned char*, nanobind::rv_policy, nanobind::detail::cleanup_list*) ()
    at /tmp/pip-build-env-wwqep4xn/overlay/lib/python3.11/site-packages/nanobind/include/nanobind/nb_func.h:105

Edit:

Crash fixed by 4b8c175.

@inducer
Copy link
Owner

inducer commented Sep 5, 2023

This seems to generally work and run loopy now. A list of to-dos remaining is in the description.

Hmm. It used to run loopy. Not sure what happened. Nvm, I misremembered.

@inducer inducer marked this pull request as ready for review September 11, 2023 02:29
@inducer inducer enabled auto-merge (rebase) September 11, 2023 02:30
@inducer inducer merged commit 981fd11 into inducer:main Sep 11, 2023
15 checks passed
@matthiasdiener matthiasdiener deleted the nanobind branch September 11, 2023 18:27
@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented Sep 11, 2023

Thank you @inducer!

Here are a few performance numbers on my M1 (all with isl-imath32):

  • microbenchmark:

    import islpy as isl
    ctx = isl.Context()
    space = isl.Space.create_from_names(isl.DEFAULT_CONTEXT, set=["x", "y"])
    
    for _ in range(1000000):
        bset = (isl.BasicSet.universe(space)
          .add_constraint(isl.Constraint.ineq_from_names(space, {1: -1, "x": 1}))
          .add_constraint(isl.Constraint.ineq_from_names(space, {1: 5, "x": -1}))
          .add_constraint(isl.Constraint.ineq_from_names(space, {1: -1, "y": 1}))
          .add_constraint(isl.Constraint.ineq_from_names(space, {1: 5, "y": -1})))
    • nanobind: 57s
    • pybind11: 85s
  • smoke_test_ks_3d total compile time:

    • nanobind: 466s
    • pybind11: 541s

This was referenced Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants