add complex array support #468

j8xixo12 · 2025-02-03T01:17:17Z

I have added complex array with SimpleArray, the complex array will be used for fourier transform.

j8xixo12 · 2025-02-03T01:29:06Z

This PR is still under development, I have problem about SimpleArray convert to numpy array.

cplx = mmComplexFloat64(1.0, 2.0)
sarr = mm.SimpleArrayComplexFloat64(10)
sarr.fill(cplx)
ndarr = np.array(sarr, copy=False, dtype=mm.ComplexFloat64.dtype())   <--- after this line, the ndarr element would treat as real = (1.0, 20) imag = 0.0 not real = 1.0, imag = 2.0.
sarr = mm.SimpleArrayComplexFloat64(array=ndarr)  <--- numpy array to SimpleArray conversion is working well.

@yungyuc @tigercosmos Do you have any idea?
I guess I must be doing something wrong with the buffer protocol.

j8xixo12 · 2025-02-03T01:30:30Z

CMakeLists.txt

@@ -78,7 +78,7 @@ endif()

 include(Flake8)

-set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD 20)


Because I have used designate initialiser, this feature is supported by c++20, therefore change it to c++ standard 20.

Is there a reason that you have to use designated initializer? Please let us know what it is.

If there is not, then just don't use designated initializer. I have no problem using it, but it alone is not a good reason to upgrade to C++20.

I just want to mark the initial value for each structure member during structure initialization, but I did not consider what side effects upgrading to C++20 may bring.

j8xixo12 · 2025-02-03T01:32:07Z

cpp/modmesh/buffer/SimpleArray.hpp

+        if constexpr (is_complex_v<value_type>)
+        {
+            initial = value_type();
+        }
+        else
+        {
+            initial = 0;
+        }


When the element type is complex, different initialization should be performed here.

j8xixo12 · 2025-02-03T01:33:51Z

cpp/modmesh/buffer/pymod/TypeBroadcast.hpp

+            constexpr bool valid_conversion = (!is_complex_v<T> && !is_complex_v<D>) || (is_complex_v<T> && is_complex_v<D> && std::is_same_v<T, D>);
+
+            if constexpr (valid_conversion)
+            {
+                arr_out.at(offset_out) = static_cast<out_type>(*ptr_in);
+            }
+            else
+            {
+                throw std::runtime_error("Cannot convert between complex and non-complex types");
+            }


Currently, only support float complex to float complex and double complex to double complex conversion.

j8xixo12 · 2025-02-03T01:37:23Z

cpp/modmesh/math/Complex.hpp

+template <typename T>
+bool operator<(const Complex<T> & lhs, const Complex<T> & rhs)
+{
+    return lhs.norm() < rhs.norm();
+}
+
+template <typename T>
+bool operator>(const Complex<T> & lhs, const Complex<T> & rhs)
+{
+    return lhs.norm() > rhs.norm();
+}


These comparison operator would be used in SimpleArray::min(), SimpleArray::max().

I think this is worth being a code comment.

I will leave a code comment for it.

j8xixo12 · 2025-02-03T01:39:04Z

cpp/modmesh/math/pymod/wrap_Complex.cpp

+            .def("dtype",
+                 []()
+                 { return py::dtype::of<wrapped_type>(); })


Add dtype getter is convenient for user to assign numpy dtype while initial numpy arrary.

tigercosmos · 2025-02-03T12:27:05Z

cpp/modmesh/buffer/SimpleArray.cpp

@@ -65,7 +66,9 @@ static std::unordered_map<std::string, DataType, DataTypeHasher> string_data_typ
    {"uint32", DataType::Uint32},
    {"uint64", DataType::Uint64},
    {"float32", DataType::Float32},
-    {"float64", DataType::Float64}};
+    {"float64", DataType::Float64},
+    {"ComplexFloat32", DataType::ComplexFloat32},


I would just call it "complex32".

I like the idea to drop Float from the complex type name, but we should use the correct bit count. For a complex number using two 32-bit float, it should be called complex64. A complex using two 64-bit float, it should be called complex128.

It follows numpy convention: https://numpy.org/doc/stable/user/basics.types.html#relationship-between-numpy-data-types-and-c-data-types

tigercosmos · 2025-02-03T12:35:12Z

gtests/test_nopython_transform.cpp

@@ -14,9 +14,9 @@ class ParsevalTest : public ::testing::Test
    const size_t VN = 1024;

    modmesh::SimpleArray<modmesh::Complex<T>> signal{
-        modmesh::small_vector<size_t>{VN}, modmesh::Complex<T>(0.0, 0.0)};
+        modmesh::small_vector<size_t>{VN}, modmesh::Complex<T>{.real_v = 0.0, .imag_v = 0.0}};


I personally don't like designated initializers

The designated initializer does not seem to be necessary.

I will remove designated initializer.

tigercosmos · 2025-02-03T12:37:14Z

regarding ndarr = np.array(sarr, copy=False, dtype=mm.ComplexFloat64.dtype()) error, it sounds like a python binding issue.

yungyuc

Stay with C++17 if possible.
Name the complex data type as complex64 and complex128, or Complex64 and Complex128, if capitalization is desired.

yungyuc · 2025-02-03T13:08:14Z

cpp/modmesh/buffer/SimpleArray.cpp

@@ -65,7 +66,9 @@ static std::unordered_map<std::string, DataType, DataTypeHasher> string_data_typ
    {"uint32", DataType::Uint32},
    {"uint64", DataType::Uint64},
    {"float32", DataType::Float32},
-    {"float64", DataType::Float64}};
+    {"float64", DataType::Float64},
+    {"ComplexFloat32", DataType::ComplexFloat32},


I like the idea to drop Float from the complex type name, but we should use the correct bit count. For a complex number using two 32-bit float, it should be called complex64. A complex using two 64-bit float, it should be called complex128.

It follows numpy convention: https://numpy.org/doc/stable/user/basics.types.html#relationship-between-numpy-data-types-and-c-data-types

yungyuc · 2025-02-03T13:10:38Z

cpp/modmesh/buffer/pymod/wrap_SimpleArrayPlex.cpp

+    {
+        if (!pybind11::isinstance<Complex<float>>(value))
+        {
+            throw pybind11::type_error("Data type mismatch, expected complex float");


The error message should use the type name.

yungyuc · 2025-02-03T13:11:02Z

cpp/modmesh/math/Complex.hpp

+template <typename T>
+bool operator<(const Complex<T> & lhs, const Complex<T> & rhs)
+{
+    return lhs.norm() < rhs.norm();
+}
+
+template <typename T>
+bool operator>(const Complex<T> & lhs, const Complex<T> & rhs)
+{
+    return lhs.norm() > rhs.norm();
+}


yungyuc · 2025-02-03T13:11:40Z

gtests/test_nopython_transform.cpp

@@ -14,9 +14,9 @@ class ParsevalTest : public ::testing::Test
    const size_t VN = 1024;

    modmesh::SimpleArray<modmesh::Complex<T>> signal{
-        modmesh::small_vector<size_t>{VN}, modmesh::Complex<T>(0.0, 0.0)};
+        modmesh::small_vector<size_t>{VN}, modmesh::Complex<T>{.real_v = 0.0, .imag_v = 0.0}};


The designated initializer does not seem to be necessary.

yungyuc · 2025-02-03T13:12:50Z

cpp/modmesh/math/Complex.hpp

@@ -40,50 +41,68 @@ struct ComplexImpl
    T real_v;


What is the rationale to append the member name with _v?

Because ComplexImpl has member function real() and imag(), so I added the _v to avoid confusion between function name and variable name.

tigercosmos · 2025-02-04T05:09:03Z

Btw, can you check if sarr = mm.SimpleArray(..., dtype="complex32") works?

j8xixo12 · 2025-02-04T16:27:58Z

Btw, can you check if sarr = mm.SimpleArray(..., dtype="complex32") works?

No, it doesn't work.

>>> sarr = mm.SimpleArray(10, dtype="complex64")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Unsupported datatype

The reason is complex64 can not be found in DataType.

And sarr = mm.SimpleArray(10, dtype="ComplexFloat64") is work.

>>> sarr = mm.SimpleArray(10, dtype="ComplexFloat64")
>>> sarr[1]
<_modmesh.ComplexFloat64 object at 0x1392238b0>
>>> sarr[1].real
0.0
>>> sarr[1].imag
0.0

add complex array support

5a02619

j8xixo12 commented Feb 3, 2025

View reviewed changes

tigercosmos reviewed Feb 3, 2025

View reviewed changes

yungyuc requested changes Feb 3, 2025

View reviewed changes

yungyuc assigned j8xixo12 Feb 3, 2025

yungyuc added the array Multi-dimensional array implementation label Feb 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add complex array support #468

add complex array support #468

j8xixo12 commented Feb 3, 2025

j8xixo12 commented Feb 3, 2025

j8xixo12 Feb 3, 2025

yungyuc Feb 3, 2025

j8xixo12 Feb 4, 2025

j8xixo12 Feb 3, 2025

j8xixo12 Feb 3, 2025

j8xixo12 Feb 3, 2025

tigercosmos Feb 3, 2025

yungyuc Feb 3, 2025

j8xixo12 Feb 4, 2025 •

edited

Loading

j8xixo12 Feb 3, 2025

tigercosmos Feb 3, 2025 •

edited

Loading

yungyuc Feb 3, 2025

tigercosmos Feb 3, 2025

yungyuc Feb 3, 2025

j8xixo12 Feb 4, 2025

tigercosmos commented Feb 3, 2025

yungyuc left a comment

yungyuc Feb 3, 2025

yungyuc Feb 3, 2025

yungyuc Feb 3, 2025

yungyuc Feb 3, 2025

yungyuc Feb 3, 2025

j8xixo12 Feb 4, 2025

tigercosmos commented Feb 4, 2025

j8xixo12 commented Feb 4, 2025

add complex array support #468

Are you sure you want to change the base?

add complex array support #468

Conversation

j8xixo12 commented Feb 3, 2025

j8xixo12 commented Feb 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j8xixo12 Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tigercosmos Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tigercosmos commented Feb 3, 2025

yungyuc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tigercosmos commented Feb 4, 2025

j8xixo12 commented Feb 4, 2025

j8xixo12 Feb 4, 2025 •

edited

Loading

tigercosmos Feb 3, 2025 •

edited

Loading