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

Switch FMM formulations to new implementation #70

Merged
merged 9 commits into from
Jan 4, 2024
Merged

Switch FMM formulations to new implementation #70

merged 9 commits into from
Jan 4, 2024

Conversation

mfschubert
Copy link
Collaborator

@mfschubert mfschubert commented Jan 2, 2024

This PR is a further step to addressing #55

  • Replace the old pol, jones, jones direct and normal implementations with new implementations that avoid iterative field optimization and instead use a single Newton step to find the optimal vector field.
  • Adjust vector field objective so that it depends upon the number of terms in the Fourier expansion, such that the field resulting from a 2x2 supercell with 4x the number of Fourier terms is identical to that of the base unit cell.

The hyperparameters (smoothness loss weight and fourier loss weight) were adjusted to achieve good convergence of simulated quantities with the number of Fourier terms in the expansion. For the uLED, this new implementation gives essentially unchanged results when compared to the earlier implementation (see figure).

image

The old implementations still remain in the code; these should be removed in a follow-up PR.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 2, 2024
@smartalecH
Copy link
Contributor

Does this resolve any of the power conservation issues you were seeing?

@mfschubert
Copy link
Collaborator Author

Does this resolve any of the power conservation issues you were seeing?

No it is unrelated. For issues related to power conservation I expect that improved s-matrix treatment will be needed.

Copy link
Contributor

@smartalecH smartalecH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mfschubert mfschubert merged commit b1fca0a into main Jan 4, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants