flaport / sax

S + Autograd + XLA :: S-parameter based frequency domain circuit simulations and optimizations using JAX.
https://flaport.github.io/sax
Apache License 2.0
63 stars 17 forks source link

`SDense` Format #40

Open jan-david-fischbach opened 1 week ago

jan-david-fischbach commented 1 week ago

Hey @flaport, there seems to be a small oddity in the way dense S-matrices are represented: The transmission from port 1 to port 2 seems to be S12. This seems to be inconsistent with the matrix expression

\mathbf{E}_\mathrm{out} = \mathbf{S} \mathbf{E}_\mathrm{in}

where $\mathbf{E}_\mathrm{in, out}$ are in the basis of the port modes. Writing this for a simple single mode two port:

\begin{bmatrix}
 \mathrm{out}_1\\\mathrm{out}_2

\end{bmatrix} = \begin{bmatrix}
 S_{11}& S_{12} \\
 S_{21}& S_{22}
\end{bmatrix} \begin{bmatrix}
 \mathrm{in}_1\\\mathrm{in}_2

\end{bmatrix}

and setting $\mathrm{in}_2=0$. We find $\mathrm{out}2=S{21}\mathrm{in}1$. This is why typically the transmission from 1->2 is contained in $S{21}$ (this is also how it is in meow).

Demo

Let's set up an SDict that contains the S matrix indices compatible to the matrix equation above.

tester = {
        ("1", "1"): 11,
        ("1", "2"): 21,
        ("2", "1"): 12,
        ("2", "2"): 22,
}

The entry with the key ("1", "2") represents the transmission from 1->2 according to the sax tutorials.

S, pm = sax.sdense(tester)
jnp.abs(S)
Array([[11., 21.],
       [12., 22.]], dtype=float64)

which is not the correct matrix indexing...

As far as I can see SDense is not used much within sax. So only minor modifications would be needed to change the convention:

The line converting from SCoo to SDense would need to be adjusted:

https://github.com/flaport/sax/blob/aabd9f9c9d8bf97c65f30cba5fe87cfac466fd84/sax/saxtypes.py#L405

to

S = S.at[..., Sj, Si].add(Sx)

Its inverse would also need to be touched:

https://github.com/flaport/sax/blob/aabd9f9c9d8bf97c65f30cba5fe87cfac466fd84/sax/saxtypes.py#L350

and in the klu backend:

https://github.com/flaport/sax/blob/aabd9f9c9d8bf97c65f30cba5fe87cfac466fd84/sax/backends/klu.py#L135

would need to be adjusted

Conclusions/Questions

It is not clear to me yet, whether this discrepancy between conventions between meow and sax has caused any hidden issues. I'll look into that.

Is there a way we can adjust the convention without introducing loads of regressions? Does it make sense to try, or do we leave it as is?

Best JD

jan-david-fischbach commented 1 week ago

I forgot to mention _sdense_to_sdict, which also needs adjustments

flaport commented 1 week ago

Hi @jan-david-fischbach ,

I think most people work with reciprocal components so I don't expect this change to impact many people (except us as meow developers probably).

Feel free to make a PR if you have the time for it :)