`SDense` Format
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
I forgot to mention _sdense_to_sdict, which also needs adjustments
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 :)