wjakob / nanobind

nanobind: tiny and efficient C++/Python bindings
BSD 3-Clause "New" or "Revised" License
2.33k stars 195 forks source link

[BUG]: problems with std::complex<long double> #708

Closed mreineck closed 2 months ago

mreineck commented 2 months ago

Problem description

I'm trying to port an existing pybind11-dependent project to nanobind, with generally good success. But I'm encountering an issue where the dtype of std::complex<long double> is taken: the compiler warns that a constant of 256 (bit size?) is cast to 0, and the subsequent unit tests fail. Here is an excerpt of the compiler output:

 x86_64-linux-gnu-g++ -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O2 -Wall -fPIC -DPKGNAME=ducc0 -DPKGVERSION=0.34.0 -I. -I./src/ -I/tmp/pip-build-env-1tjgqhli/overlay/local/lib/python3.12/dist-packages/nanobind/include -I/usr/include/python3.12 -c python/ducc.cc -o build/temp.linux-x86_64-cpython-312/python/ducc.o -std=c++17 -fvisibility=hidden -g0 -ffast-math -O3 -march=native -Wfatal-errors -Wfloat-conversion -W -Wall -Wstrict-aliasing -Wwrite-strings -Wredundant-decls -Woverloaded-virtual -Wcast-qual -Wcast-align -Wpointer-arith -Wnon-virtual-dtor -Wzero-as-null-pointer-constant -pthread
  In file included from /tmp/pip-build-env-1tjgqhli/overlay/local/lib/python3.12/dist-packages/nanobind/include/nanobind/nanobind.h:46,
                   from python/ducc.cc:15:
  /tmp/pip-build-env-1tjgqhli/overlay/local/lib/python3.12/dist-packages/nanobind/include/nanobind/nb_types.h: In constructor ‘nanobind::handle::handle(const PyObject*)’:
  /tmp/pip-build-env-1tjgqhli/overlay/local/lib/python3.12/dist-packages/nanobind/include/nanobind/nb_types.h:182:51: warning: cast from type ‘const PyObject*’ {aka ‘const _object*’} to type ‘PyObject*’ {aka ‘_object*’} casts away qualifiers [-Wcast-qual]
    182 |     NB_INLINE handle(const PyObject *ptr) : m_ptr((PyObject *) ptr) { }
        |                                                   ^~~~~~~~~~~~~~~~
  /tmp/pip-build-env-1tjgqhli/overlay/local/lib/python3.12/dist-packages/nanobind/include/nanobind/nb_types.h: In constructor ‘nanobind::handle::handle(const PyTypeObject*)’:
  /tmp/pip-build-env-1tjgqhli/overlay/local/lib/python3.12/dist-packages/nanobind/include/nanobind/nb_types.h:183:55: warning: cast from type ‘const PyTypeObject*’ {aka ‘const _typeobject*’} to type ‘PyObject*’ {aka ‘_object*’} casts away qualifiers [-Wcast-qual]
    183 |     NB_INLINE handle(const PyTypeObject *ptr) : m_ptr((PyObject *) ptr) { }
        |                                                       ^~~~~~~~~~~~~~~~
  In file included from ./python/fft_pymod.cc:40,
                   from python/ducc.cc:17:
  /tmp/pip-build-env-1tjgqhli/overlay/local/lib/python3.12/dist-packages/nanobind/include/nanobind/ndarray.h: In instantiation of ‘constexpr nanobind::dlpack::dtype nanobind::dtype() [with T = std::complex<long double>]’:
  ./python/fft_pymod.cc:169:37:   required from here
    169 |     ||(a.dtype() == py::dtype<clong>()))
        |                     ~~~~~~~~~~~~~~~~^~
  /tmp/pip-build-env-1tjgqhli/overlay/local/lib/python3.12/dist-packages/nanobind/include/nanobind/ndarray.h:129:29: warning: conversion from ‘long unsigned int’ to ‘uint8_t’ {aka ‘unsigned char’} changes value from ‘256’ to ‘0’ [-Woverflow]
    129 |     result.bits = sizeof(T) * 8;
        |                   ~~~~~~~~~~^~~

Is this something that should be working, or am I using nanobind incorrectly? I'm happy to work on a small reproducer if this is helpful.

Reproducible example code

No response

wjakob commented 2 months ago

What would you expect nanobind to do with a long double? That type is inherently non-portable. Python does not provide functionality to convert a C long double into a Python float, or vice versa. The only thing one could do is to drop the long part silently, which seems bug-prone and a bad default behavior.

mreineck commented 2 months ago

Well, there is numpy.longdouble ... I know that it is not guaranteed to be any longer than numpy.double, but it worked flawlessly with pybind11 whenever the type existed on the target architecture. In the big picture it's not something I absolutely need in my applications, but scipy.fft, for example, relies on this feature, and that would mean that pocketfft (which is used internally by scipy) cannot be ported to nanobind.

Quick example (on x86_64):

import numpy as np
import scipy
print (scipy.fft.fftn(np.zeros(10,dtype=np.longdouble)).dtype)

This will print complex256.

wjakob commented 2 months ago

There is a big difference between nanobind and pybind11 here:

pybind11 had NumPy integration. For this, it communicated (at runtime) with the low-level NumPy API, which in turn required NumPy to be installed.

nanobind does not have a dependency on any particular Tensor API. It implements a generic ndarray type that can talk to many different libraries, but without making any assumption about such low-level bits. A long double unfortunately cannot be passed through these interfaces.

You will need to create a wrapper that casts numeric types before binding them with nanobind. I will close this issue, since there isn't an obvious solution.

mreineck commented 2 months ago

Thanks for the explanation!