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

Default group_index_step to True in ModeSpec #1169

Closed
tomflexcompute opened this issue Sep 22, 2023 · 10 comments
Closed

Default group_index_step to True in ModeSpec #1169

tomflexcompute opened this issue Sep 22, 2023 · 10 comments
Assignees

Comments

@tomflexcompute
Copy link
Contributor

A few users have been confused when using the mode solver and get none from extracting the group index. Shall we set group_index_step to True by default?

@momchil-flex
Copy link
Collaborator

One thing that comes to mind is that it may make sense to have it True in ModeSolver and maybe in ModeSolverMonitor, but False in ModeMonitor to avoid extra simulation time (it's not really the purpose of this monitor to compute the group index). However, it is defined in ModeSpec for all of those, so changing the default there will change it for all.

@tylerflex @lucas-flexcompute any comments?

@lucas-flexcompute
Copy link
Collaborator

TBH I'm more in favor of leaving the default False everywhere. Group index is costly (3× the credits) and, in most cases, not required. I think that was the original reason for leaving it off by default.

@tylerflex
Copy link
Collaborator

I'm also in favor of not changing default behavior if possible, simply for backwards compatibility. Is there a way to improve the warning messaging to explain what's going on when group_index is returned as None? for example?

@lucas-flexcompute
Copy link
Collaborator

I'm also in favor of not changing default behavior if possible, simply for backwards compatibility. Is there a way to improve the warning messaging to explain what's going on when group_index is returned as None? for example?

Yeah, I think a warning message when that happens is more useful (we can probably make it a "single-issue" warning as well).

lucas-flexcompute added a commit that referenced this issue Sep 26, 2023
Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute added a commit that referenced this issue Sep 26, 2023
Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute added a commit that referenced this issue Sep 26, 2023
Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute added a commit that referenced this issue Sep 27, 2023
Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
@dbochkov-flexcompute
Copy link
Contributor

dbochkov-flexcompute commented Oct 2, 2023

@weiliangjin2021 and I are chatted sometime ago about the possibility to compute group index without using finite differences: see equation (14) in https://math.mit.edu/~stevenj/papers/LohOs09.pdf. Do you think it's something we can adopt to make calculations faster? I assume there could be difficulties with angled, fully anisotropic or lossy?

@momchil-flex
Copy link
Collaborator

Interesting! It could be worth a try since it should be quite quick. My main worry is if the two different ways to obtain the result don't match up exactly (due to e.g. some numerical differences).

Actually when Lucas first started working on adding the group index computation, I got interested in whether it can be done by computing both n(f) and dn/df simultaneously. This can be done e.g. by a complex number trick inspired by automatic differentiation. If you extend your function n(f) to be defined over the complex numbers, then n(f + 1i * eps) = n(f) + 1i * eps * dn/df + O(eps**2), and so by computing n(f + 1i * eps) you get n as the real part and dn/df as the imaginary part. I only gave this a quick try though as there are some complications (making everything work with imaginary frequency, defining n as the real part of the eigenvalue even though you want your n(f + 1i * eps) to be complex, ...) It was more out of curiosity really, since in typical cases it would probably not even be that much more efficient, as it would make a real eigenproblem complex.

But anyway, good to try your idea I think!

@lucas-flexcompute
Copy link
Collaborator

I'm under the impression that eq. 14 is only valid for non-dispersive materials, and that's probably one of the main applications of group velocity calculations. Is there any easy and accurate correction factor that we can use to include dispersion?

@weiliangjin2021
Copy link
Collaborator

I'm under the impression that eq. 14 is only valid for non-dispersive materials, and that's probably one of the main applications of group velocity calculations. Is there any easy and accurate correction factor that we can use to include dispersion?

There is Eq.15 for dispersive medium. To generalize to lossy medium, the conjugated inner product needs to be replaced with unconjugated one. But I'm not sure if any other changes are needed.

@lucas-flexcompute
Copy link
Collaborator

I see. If that's easy to implement, I don't see why not. It would end all of the issues with group index calculation!

lucas-flexcompute added a commit that referenced this issue Oct 10, 2023
Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
lucas-flexcompute added a commit that referenced this issue Oct 10, 2023
…1178)

Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
@lucas-flexcompute
Copy link
Collaborator

Merged in #1178

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

6 participants