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

SDense Format #40

Open
jan-david-fischbach opened this issue Oct 17, 2024 · 2 comments
Open

SDense Format #40

jan-david-fischbach opened this issue Oct 17, 2024 · 2 comments

Comments

@jan-david-fischbach
Copy link
Contributor

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:

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

to

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

Its inverse would also need to be touched:

Sj, Si = jnp.meshgrid(jnp.arange(S.shape[-1]), jnp.arange(S.shape[-2]))

and in the klu backend:

CextT_S_inv_I_CS_Cext = S_inv_I_CS_Cext[..., Cexti, :][..., :, Cextj]

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
Copy link
Contributor Author

I forgot to mention _sdense_to_sdict, which also needs adjustments

@flaport
Copy link
Owner

flaport commented Oct 21, 2024

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 :)

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