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

2nd round of autograd fixes #1923

Merged
merged 3 commits into from
Sep 3, 2024
Merged

2nd round of autograd fixes #1923

merged 3 commits into from
Sep 3, 2024

Conversation

tylerflex
Copy link
Collaborator

@tylerflex tylerflex commented Aug 27, 2024

  • some adjoint source fixes for FieldData involving H fields.
  • better gradients for Box shifting boundaries based on permittivity.
  • re-introduce automatic permittivity grabbing for geometry group shifting boundaries.
  • better error and warning handling across the board.
  • adds a way to visualize adjoint fields (only for development purposes currently, no public API).

@tylerflex tylerflex marked this pull request as draft August 27, 2024 16:58
@tylerflex tylerflex added 2.7 will go into version 2.7.* .3 labels Aug 27, 2024
@tylerflex tylerflex force-pushed the tyler/autograd_/cleanup2 branch 2 times, most recently from 0a20211 to b74f1a3 Compare August 30, 2024 16:54
@tylerflex tylerflex marked this pull request as ready for review August 30, 2024 16:54
Copy link
Contributor

@yaugenst-flex yaugenst-flex left a comment

Choose a reason for hiding this comment

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

This is great. I've been testing box and hfield changes and things seem to work much better now.

@@ -471,7 +471,7 @@ def field_vol_postprocess_fn(sim_data, mnt_data):
def field_point_postprocess_fn(sim_data, mnt_data):
value = 0.0
for _, val in mnt_data.field_components.items():
value += abs(anp.sum(val.values))
value += abs(anp.sum(abs(val.values)))
Copy link
Contributor

Choose a reason for hiding this comment

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

double abs? not that it matters i guess

Copy link
Contributor

Choose a reason for hiding this comment

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

Could be nice to have private debug run function, or maybe a @debug decorator that could do things like this? Not really important but might be convenient to include in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

which line? the adjoint field visualization?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah exactly, that and the global toggle to trigger that code


# make source go backwards
if "H" in name:
values *= -1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this what we do in the adjoint plugin too?

I am trying to think about whether this is the correct thing to do in all cases. It's probably possible to see it somehow from the adjoint formulation if written for the H field. I think this is correct. I'm just wondering a little because the intuition of this minus sign "making source go backwards" makes sense for a real propagating mode, but it gets a bit murkier for evanescent or lossy modes. The other option is to take np.conj(values). If the E field is real and the mode is lossless and propagating, then the H field is imaginary and the two operations are the same. In general the np.conj(values) operation would be like time reversal while the values *= -1 operation is like reflection of the mode in the source plane, which produce different results in some cases.

I do think the -1 is correct here though but do you remember if it comes out of the adjoint math?

@tylerflex tylerflex merged commit 08dc2be into develop Sep 3, 2024
15 checks passed
@tylerflex tylerflex deleted the tyler/autograd_/cleanup2 branch September 3, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.7 will go into version 2.7.* .3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants