qiboteam / qibotn

The tensor-network translation module for Qibo.
https://qibo.science
Apache License 2.0
3 stars 1 forks source link

Pytest on cuquantum evaluation failed when using qibo-0.2.2, python 3.11. #21

Closed liweintu closed 6 months ago

liweintu commented 7 months ago

The failure is caused by the mismatch between the dtype of the state vector result and that of tensor network result, triggered by line-48 in test_cuquantum_cutensor_backend.py, as follows. assert np.allclose( cp.array(result_sv), result_tn), "Resulting dense vectors do not match"

OS: Ubuntu 20.04.6 LTS Python: 3.11 Qibo: 0.2.2

Error message below. ../../../miniforge3/envs/cuquantum_py311/lib/python3.11/site-packages/cupy/_creation/from_data.py:46: in array return _core.array(obj, dtype, copy, order, subok, ndmin) cupy/_core/core.pyx:2376: in cupy._core.core.array ??? cupy/_core/core.pyx:2400: in cupy._core.core.array ???


??? E ValueError: Unsupported dtype object

liweintu commented 7 months ago

type(result_sv) is of numpy.ndarray, and type(result_tn) is of copy.ndarray.

However, the shape of result_sv is (), with dtype being of 'numpy.dtypes.ObjectDType', whereas the shape of result_tn is (2,), with dtype being complex128.

Therefore, the mismatch in dtypes (and possibly in shape) causes the error.

liweintu commented 7 months ago

A possible fix could be, we uniform the dtype so that result_sv and result_tn can be compared.

@AleCandido Which dtype do you recommend we can convert to? numpy.dtypes.ObjectDType or complex128

alecandido commented 7 months ago

type(result_sv) is of numpy.ndarray, and type(result_tn) is of copy.ndarray.

I guess here you meant cupy.ndarray.

However, the shape of result_sv is (), with dtype being of 'numpy.dtypes.ObjectDType', whereas the shape of result_tn is (2,), with dtype being complex128.

Therefore, the mismatch in dtypes (and possibly in shape) causes the error.

The mismatch in dtypes might not cause the error, e.g.

In [1]: import numpy as np

In [2]: np.allclose(np.arange(10, dtype=np.int16), np.arange(10, dtype=np.int64))
Out[2]: True

but it would be better to standardize in any case.

@AleCandido Which dtype do you recommend we can convert to? numpy.dtypes.ObjectDType or complex128

numpy.dtypes.ObjectDType is a bad sign: something is being treated as a Python object, rather than a NumPy-builtin number type. For us, they should all be arrays of numbers, so they should have suitable numeric dtypes. If the choice has been complex128, then let's keep this one.

In general, it might be relevant also to expose the dtype choice to the user, to use less memory if less precision is required, and these kinds of considerations. But this could happen at a later stage.

alecandido commented 7 months ago

Therefore, the mismatch in dtypes (and possibly in shape) causes the error.

Actually, the shape is for sure a problem. I'm not sure why the _sv kind is empty. This should be investigated.

liweintu commented 7 months ago

type(result_sv) is of numpy.ndarray, and type(result_tn) is of copy.ndarray.

I guess here you meant cupy.ndarray.

Yup, my bad for the typo. It's cupy.ndarray.

However, the shape of result_sv is (), with dtype being of 'numpy.dtypes.ObjectDType', whereas the shape of result_tn is (2,), with dtype being complex128. Therefore, the mismatch in dtypes (and possibly in shape) causes the error.

The mismatch in dtypes might not cause the error, e.g.

In [1]: import numpy as np

In [2]: np.allclose(np.arange(10, dtype=np.int16), np.arange(10, dtype=np.int64))
Out[2]: True

but it would be better to standardize in any case.

@AleCandido Which dtype do you recommend we can convert to? numpy.dtypes.ObjectDType or complex128

numpy.dtypes.ObjectDType is a bad sign: something is being treated as a Python object, rather than a NumPy-builtin number type. For us, they should all be arrays of numbers, so they should have suitable numeric dtypes. If the choice has been complex128, then let's keep this one.

Good. Being an ndarray with complex128 is also good so that we can re-use numpy/cupy's comparison APIs.

In general, it might be relevant also to expose the dtype choice to the user, to use less memory if less precision is required, and these kinds of considerations. But this could happen at a later stage.

Yup, good point.

liweintu commented 7 months ago

Therefore, the mismatch in dtypes (and possibly in shape) causes the error.

Actually, the shape is for sure a problem. I'm not sure why the _sv kind is empty. This should be investigated.

It's not empty, but a scalar. If I print it out, this is its shape and value inside. print(result_sv.shape, result_sv) () (0.70711+0j)|0> + (0.70711+0j)|1>

Quick question. Do you know where the class is defined for this Python object? I'd like to translate the Python object into an array of complex128. I found some classes for results in Qibo, as below, but this Python object doesn't seem to be any of them. image

alecandido commented 7 months ago

It's not empty, but a scalar. If I print it out, this is its shape and value inside. print(result_sv.shape, result_sv) () (0.70711+0j)|0> + (0.70711+0j)|1>

Ok, I see. I imagined that something like that was happening... just after writing my reply.

The idea is that you essentially have a scalar "array" (a NumPy object with a single element) essentially containing an object that contains in turn an array (two elements this time, the two complex numbers represented in the string).

alecandido commented 7 months ago

I believe the reason of this to be here: https://github.com/qiboteam/qibotn/blob/918958aae191eac4687f85e302e47dd613aa0f2f/tests/test_cuquantum_cutensor_backend.py#L12-L13

Most likely, you're using a new version of Qibo, that it's returning an object as a result, instead of an array (that probably was being returned before).

And calling np.array() on it, NumPy doesn't know that inside the object there are actually two elements (most likely already complex).

So, you should extract the array-like from the Qibo result before wrapping in NumPy (and maybe you won't even need to wrap).

liweintu commented 7 months ago

It's not empty, but a scalar. If I print it out, this is its shape and value inside. print(result_sv.shape, result_sv) () (0.70711+0j)|0> + (0.70711+0j)|1>

Ok, I see. I imagined that something like that was happening... just after writing my reply.

The idea is that you essentially have a scalar "array" (a NumPy object with a single element) essentially containing an object that contains in turn an array (two elements this time, the two complex numbers represented in the string).

Exactly.

liweintu commented 7 months ago

I believe the reason of this to be here:

https://github.com/qiboteam/qibotn/blob/918958aae191eac4687f85e302e47dd613aa0f2f/tests/test_cuquantum_cutensor_backend.py#L12-L13

Most likely, you're using a new version of Qibo, that it's returning an object as a result, instead of an array (that probably was being returned before).

And calling np.array() on it, NumPy doesn't know that inside the object there are actually two elements (most likely already complex).

So, you should extract the array-like from the Qibo result before wrapping in NumPy (and maybe you won't even need to wrap).

Yeah. The older version of Qibo doesn't have this issue. I'm on Qibo 0.2.2, which incurs this. @nitinshivaraman also finds this issue in Quimb after upgrading to the new Qibo version.

If the returned value is supposed to be an array of complex items rather than an object, do you think it makes more sense to modify Qibo to return the array rather than to modify both cuQuantum and Quimb's test code to do the translation? I'm a bit inclined to the former so that we can standardise the array-like representation.

alecandido commented 7 months ago

If the returned value is supposed to be an array of complex items rather than an object, do you think it makes more sense to modify Qibo to return the array rather than to modify both cuQuantum and Quimb's test code to do the translation? I'm a bit inclined to the former so that we can standardise the array-like representation.

I believe Qibo has to account for many different results, that's why the object has been implemented.

https://github.com/qiboteam/qibo/pull/1039

So, I would suggest extracting the result you're interested in here, rather than attempting to change Qibo again (even because we should rediscuss what we want in the general case).

liweintu commented 7 months ago

If the returned value is supposed to be an array of complex items rather than an object, do you think it makes more sense to modify Qibo to return the array rather than to modify both cuQuantum and Quimb's test code to do the translation? I'm a bit inclined to the former so that we can standardise the array-like representation.

I believe Qibo has to account for many different results, that's why the object has been implemented.

qiboteam/qibo#1039

So, I would suggest extracting the result you're interested in here, rather than attempting to change Qibo again (even because we should rediscuss what we want in the general case).

Ok, following this idea, here's a quick fix.

As below, just replace the np.array() with .state() to convert the returned QuantumState object to a numpy array instead, then pytest will pass.

def qibo_qft(nqubits, swaps):
    circ_qibo = QFT(nqubits, swaps)
    # state_vec = np.array(circ_qibo())
    state_vec = circ_qibo().state(numpy=True)
alecandido commented 6 months ago

@liweintu wasn't this closed by #22?

liweintu commented 6 months ago

@liweintu wasn't this closed by #22?

Oh yeah. It's done in #22 . Closing this issue now. Thanks for the reminder.