Implement simple vector and tensor types #735

Jullija · 2025-10-26T14:16:57Z

Added

Types

cartesian_vector — fixed 3D vector
cartesian_tensor<T,R,C> — matrix/tensor with R,C ∈ [1..3]

Vector ops (a,b : cartesian_vector ; s : scalar)

a + b — elementwise addition
a - b — elementwise subtraction
a % b — elementwise modulo (% for integral, fmod for floating)
a * s, s * a — scalar multiply
a / s — scalar divide
a ⋅ b (scalar_product, alias dot) — dot product → scalar
a × b (vector_product, alias cross) — cross product → vector
|a| (magnitude, norm) — Euclidean length (floating reps only)
a/|a| (unit) — unit vector (floating reps only)

Tensor ops (A,B : cartesian_tensor<R,C> ; M:N, matching dims; s: scalar)

A + B, A - B — elementwise
A % B — elementwise modulo (% / fmod as above)
A * s, s * A, A / s — scalar scale
A · x (matvec) — 3×3 tensor–vector product → vector
A · B (matmul) — matrix multiply (R×K)*(K×C) → (R×C)
A : B (double_contraction) — scalar (sum of elementwise products)
a ⊗ b (outer_numeric) — vector outer product → 3×3 tensor

mpusz

Hi!

Thanks for contributing! I had time to review only cartesian_vector changes and provided some feedback that I believe will also be meaningful to another type as well.

mpusz · 2025-10-26T17:03:21Z

src/core/include/mp-units/cartesian_vector.h

-#include <mp-units/bits/requires_hosted.h>
-//
 #include <mp-units/bits/module_macros.h>
+#include <mp-units/bits/requires_hosted.h>


We put requires_hosted.h as the first header in all HOSTED implementations. Do not use alphabetical order for this header.

Still left to resolve.

mpusz · 2025-10-26T17:04:56Z

src/core/include/mp-units/cartesian_vector.h

 namespace mp_units {

-MP_UNITS_EXPORT template<detail::Scalar T>
+MP_UNITS_EXPORT template<detail::Scalar T = double>


It is better to provide template defaults in the implementation rather than in one of (possibly many) forward declarations.

mpusz · 2025-10-26T17:05:51Z

src/core/include/mp-units/cartesian_vector.h

-    requires requires(T t, U u) { t + u; }
+    requires requires(const T& t, const U& u) { t + u; }


Does it make any difference?

I've read that adding const doesn't mutate an input which is desired in arithmetic operations (https://en.cppreference.com/w/cpp/language/operator_arithmetic.html)

You are right: https://godbolt.org/z/3zW8ns6YP. Thanks!

mpusz · 2025-10-26T17:06:18Z

src/core/include/mp-units/cartesian_vector.h

 namespace detail {

 struct cartesian_vector_iface {
+  // A + B


Such comments do not improve anything.

mpusz · 2025-10-26T17:10:32Z

src/core/include/mp-units/cartesian_vector.h

+  // A % B  (integral: %, floating: fmod into CT)
  template<typename T, typename U>
-    requires requires(T t, U u) { t * u; }
-  [[nodiscard]] friend constexpr auto operator*(const cartesian_vector<T>& lhs, const U& rhs)
+    requires(requires(const T& t, const U& u) { t % u; }) || (std::floating_point<T> && std::floating_point<U>)
+  [[nodiscard]] friend constexpr auto operator%(const cartesian_vector<T>& lhs, const cartesian_vector<U>& rhs)
+  {
+    using CT = std::common_type_t<T, U>;
+    if constexpr (std::floating_point<T> && std::floating_point<U>) {
+      using std::fmod;
+      return ::mp_units::cartesian_vector<CT>{static_cast<CT>(fmod(static_cast<long double>(lhs._coordinates_[0]),
+                                                                   static_cast<long double>(rhs._coordinates_[0]))),
+                                              static_cast<CT>(fmod(static_cast<long double>(lhs._coordinates_[1]),
+                                                                   static_cast<long double>(rhs._coordinates_[1]))),
+                                              static_cast<CT>(fmod(static_cast<long double>(lhs._coordinates_[2]),
+                                                                   static_cast<long double>(rhs._coordinates_[2])))};
+    } else {
+      return ::mp_units::cartesian_vector<CT>{static_cast<CT>(lhs._coordinates_[0] % rhs._coordinates_[0]),
+                                              static_cast<CT>(lhs._coordinates_[1] % rhs._coordinates_[1]),
+                                              static_cast<CT>(lhs._coordinates_[2] % rhs._coordinates_[2])};
+    }
+  }


There is no op% for floating-point types for a reason. We should behave like fundamental types, so it should not be available as well for floating-point representation types. If we care about modulo for such types, we should provide fmod non-member function. See fmod from math.h for more info.

mpusz · 2025-10-26T17:27:15Z

src/core/include/mp-units/cartesian_vector.h

-  template<typename FormatContext>
-  auto format(const mp_units::cartesian_vector<T>& vec, FormatContext& ctx) const
+  template<typename Ctx>
+  auto format(const mp_units::cartesian_vector<T>& v, Ctx& ctx) const
  {
-    return format_to(ctx.out(), "[{}, {}, {}]", vec[0], vec[1], vec[2]);
+    return format_to(ctx.out(), "[{}, {}, {}]", v[0], v[1], v[2]);


Please restore the previous code. FormatContext provides more info and is used consistently in the entire project for all formatters.

v is too short as an identifier and often causes some stupid shadowing errors in MSVC.

mpusz · 2025-10-26T17:29:19Z

test/runtime/cartesian_vector_test.cpp

 import mp_units;
 #else
 #include <mp-units/cartesian_vector.h>
+#if MP_UNITS_HOSTED


Runtime tests are only run for HOSTED so no need to check here.

mpusz · 2025-10-26T17:30:22Z

test/runtime/cartesian_vector_test.cpp

+    REQUIRE(v[0] == 0);
+    REQUIRE(v[1] == 0);
+    REQUIRE(v[2] == 0);


Should be garbage

This is why I didn't provide default construction.

mpusz · 2025-10-26T17:31:15Z

test/runtime/cartesian_vector_test.cpp

+
+  SECTION("zero arguments")
+  {
+    cartesian_vector<double> v{};


This is how zeros should be set, but this collides with your default-constructor now

mpusz · 2025-10-26T17:36:59Z

test/runtime/cartesian_vector_test.cpp

-using namespace Catch::Matchers;
+using Catch::Matchers::WithinAbs;
+
+TEST_CASE("cartesian_vector initialization and access", "[vector]")


I personally prefer to have fewer TEST_CASES in catch, so it is easier to manage. For example, if I want to check if all vector tests still work, it can check the status of one easy-to-find test rather than searching for many different strings.

Filtering by a tag is not often an option.

Jullija · 2025-10-26T18:53:11Z

Hi! Thank you so much! I'll start reading and implementing changes - thank you and sorry in advance for any problems! :)

mpusz · 2025-10-26T21:09:54Z

src/core/include/mp-units/cartesian_vector.h

+  cartesian_vector() = default;
  cartesian_vector(const cartesian_vector&) = default;
  cartesian_vector(cartesian_vector&&) = default;
  cartesian_vector& operator=(const cartesian_vector&) = default;
  cartesian_vector& operator=(cartesian_vector&&) = default;

  template<typename... Args>
-    requires(... && std::constructible_from<T, Args>)
+    requires(sizeof...(Args) <= 3) && (... && std::constructible_from<T, Args>)
  constexpr explicit(!(... && std::convertible_to<Args, T>)) cartesian_vector(Args&&... args) :
      _coordinates_{static_cast<T>(std::forward<Args>(args))...}
  {
  }

  template<typename U>
-    requires std::constructible_from<T, U>
+    requires std::constructible_from<T, const U&>
  constexpr explicit(!std::convertible_to<U, T>) cartesian_vector(const cartesian_vector<U>& other) :
      _coordinates_{static_cast<T>(other[0]), static_cast<T>(other[1]), static_cast<T>(other[2])}
  {
  }

  template<typename U>
-    requires std::constructible_from<T, U>
+    requires std::constructible_from<T, U&&>
  constexpr explicit(!std::convertible_to<U, T>) cartesian_vector(cartesian_vector<U>&& other) :
      _coordinates_{static_cast<T>(std::move(other[0])), static_cast<T>(std::move(other[1])),
                    static_cast<T>(std::move(other[2]))}


Let's remove all of those. This will result in an aggregate type (such as std::array), which should solve all our issues. Please, change the comment next to a public member to reflect that.

Jullija · 2025-11-11T07:35:19Z

@mpusz could you help me fix the pipeline? I tried in some many ways and I have no idea how to fix it :c

mpusz · 2025-11-12T17:35:27Z

I am currently on vacation in Hawaii, so I don't have much time to look into it. I will check it out when I am back.

mpusz · 2025-12-20T12:32:04Z

src/core/include/mp-units/cartesian_vector.h

-#include <mp-units/bits/requires_hosted.h>
-//
 #include <mp-units/bits/module_macros.h>
+#include <mp-units/bits/requires_hosted.h>


Still left to resolve.

mpusz · 2025-12-20T12:32:30Z

src/core/include/mp-units/cartesian_tensor.h

+#pragma once
+
+#include <mp-units/bits/module_macros.h>
+#include <mp-units/bits/requires_hosted.h>


Should be the first include

mpusz · 2025-12-26T18:19:46Z

src/core/include/mp-units/cartesian_vector.h

+MP_UNITS_EXPORT template<typename T, typename U>
+  requires requires(const T& t, const U& u) { t + u; }
+[[nodiscard]] constexpr auto operator+(const cartesian_vector<T>& lhs, const cartesian_vector<U>& rhs)
+{
+  return ::mp_units::cartesian_vector{lhs._coordinates_[0] + rhs._coordinates_[0],
+                                      lhs._coordinates_[1] + rhs._coordinates_[1],
+                                      lhs._coordinates_[2] + rhs._coordinates_[2]};
+}


All of the operators and other customization points should be hidden friends as they were before.

mpusz · 2025-12-26T18:23:26Z

test/runtime/cartesian_vector_test.cpp

    {
-      cartesian_vector<double> v2 = v1;
+      cartesian_vector v1{1.0, 2.0, 3.0};
+      cartesian_vector<double> v2 = v1;  // Use initialization instead of assignment


This is not an assignment

mpusz · 2025-12-26T18:25:24Z

test/static/quantity_test.cpp

 static_assert(std::constructible_from<cartesian_vector<double>, quantity<one, cartesian_vector<double>>>);
 static_assert(!std::convertible_to<quantity<one, double>, cartesian_vector<double>>);
-static_assert(std::constructible_from<cartesian_vector<double>, quantity<one, double>>);
+static_assert(std::is_aggregate_v<cartesian_vector<double>>);


This change makes no sense and removes proper test

mpusz · 2025-12-26T18:26:10Z

test/runtime/cartesian_vector_test.cpp

 #else
 #include <mp-units/cartesian_vector.h>
+#include <mp-units/ext/format.h>
+#include <sstream>


Included twice

mpusz · 2025-12-26T18:26:21Z

test/runtime/cartesian_vector_test.cpp

+#endif
+
+#ifndef MP_UNITS_MODULES
+#include <sstream>


import std; missing.

mpusz · 2025-12-26T18:30:14Z

test/runtime/cartesian_vector_test.cpp

+    REQUIRE_THAT(u.magnitude(), WithinAbs(1.0, 1e-12));
+    REQUIRE_THAT(u[0], WithinAbs(3.0 / 5.0, 1e-12));
+    REQUIRE_THAT(u[1], WithinAbs(4.0 / 5.0, 1e-12));
+    REQUIRE_THAT(u[2], WithinAbs(0.0, 1e-12));


Use AlmostEquals instead

mpusz · 2025-12-26T18:32:15Z

test/runtime/cartesian_vector_test.cpp

  }
-}
-
-TEST_CASE("cartesian_vector text output", "[vector][fmt][ostream]")


Why did you remove my tests that were testing each text output separately for different representation types?

Julia Smerdel and others added 5 commits October 11, 2025 21:31

add % operation and aliases

edec240

add header with vector and tensors

ab3c2e8

spli into 2 files

a4f0682

add tests

73d8554

small changes

fa57a3a

Jullija changed the title ~~Implement simple vector and tensor types~~ [DRAFT] Implement simple vector and tensor types Oct 26, 2025

Jullija marked this pull request as draft October 26, 2025 16:29

formatting

7bc1e18

Jullija force-pushed the feature/la-vectors branch from 20a6db0 to 7bc1e18 Compare October 26, 2025 16:33

mpusz requested changes Oct 26, 2025

View reviewed changes

vector fixes after review

90a371c

mpusz requested changes Oct 26, 2025

View reviewed changes

Jullija added 2 commits November 2, 2025 14:57

fix tensor and vector after review

26c88fe

fix tensor test

00b2e74

Jullija force-pushed the feature/la-vectors branch 9 times, most recently from f3f2391 to 00b2e74 Compare November 10, 2025 20:46

Jullija force-pushed the feature/la-vectors branch 2 times, most recently from 1618ca3 to e564751 Compare November 20, 2025 18:41

fix pipeline by adding export

59f9298

Jullija force-pushed the feature/la-vectors branch from e564751 to 59f9298 Compare November 20, 2025 18:52

Jullija changed the title ~~[DRAFT] Implement simple vector and tensor types~~ Implement simple vector and tensor types Nov 20, 2025

format change

1ecb38f

Jullija force-pushed the feature/la-vectors branch from 3e231f3 to 1ecb38f Compare November 20, 2025 19:19

Merge branch 'master' into feature/la-vectors

40fb1ee

Jullija marked this pull request as ready for review November 20, 2025 19:21

Jullija requested a review from mpusz November 20, 2025 19:38

mpusz requested changes Dec 26, 2025

View reviewed changes

		requires requires(T t, U u) { t + u; }
		requires requires(const T& t, const U& u) { t + u; }

Uh oh!

Implement simple vector and tensor types #735

Are you sure you want to change the base?

Implement simple vector and tensor types #735

Uh oh!

Conversation

Jullija commented Oct 26, 2025

Uh oh!

mpusz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jullija commented Oct 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jullija commented Nov 11, 2025

Uh oh!

mpusz commented Nov 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants