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

Vector times array #173

Open
adrn opened this issue Aug 30, 2024 · 4 comments
Open

Vector times array #173

adrn opened this issue Aug 30, 2024 · 4 comments

Comments

@adrn
Copy link

adrn commented Aug 30, 2024

Currently, this does not work:

>>> q = Quantity([1, 2, 3], "kpc")
>>> vec = cx.CartesianPosition3D.constructor(q)
>>> scale = jnp.array([1, 0.9, 0.85])
>>> vec * scale
EquinoxTracetimeError: ...

I think this should be supported when the shape of a multiplicative array is broadcastable with the vector shape, and it should do the same as q * scale.

(thinking about this as I work on a PR for a new AnisotropicScalingOperator)

@nstarman
Copy link
Contributor

nstarman commented Aug 30, 2024

This is meant to be a shorthand for $\mathbf{v{\prime}} = S \mathbf{v}$ where

$$ S = \begin{bmatrix} s_1 & 0 & \cdots & 0 \\ 0 & s_2 & \cdots & 0 \\ \vdots & \vdots & \ddots & \vdots \\ 0 & 0 & \cdots & s_n \end{bmatrix} $$

I'm fully in support of explicit matrix-vector multiplication.

scale = jnp.array([1, 0.9, 0.85])
jnp.diag(scale) @ vector

IMO less so for implicit m-v multiplication when it looks like a scalar-vector multiplication.
For that, we should defer to existing numpy functions, e.g. numpy.matmul() semantics.

@adrn
Copy link
Author

adrn commented Aug 30, 2024

I hear you, but it seems like a waste to fill out a 2D matrix with zeros for a simple diagonal scaling operation! Vectors have a .norm() -- what about a .scale() like def scale(self, scale_factors: Array) -> PosT and etc.?

@nstarman
Copy link
Contributor

That seems like a good workaround! We get to keep the mathematical rigour in multiplication, but offer the same convenience through a specific method

@nstarman
Copy link
Contributor

On a related note, sparse array support would be useful here!
https://jax.readthedocs.io/en/latest/jax.experimental.sparse.html

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

No branches or pull requests

2 participants