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

WIP: Meepgeom refactoring #1754

Merged
merged 7 commits into from
Oct 1, 2021
Merged

Conversation

smartalecH
Copy link
Collaborator

Closes #1649.

@smartalecH smartalecH marked this pull request as draft September 7, 2021 20:46
@stevengj
Copy link
Collaborator

stevengj commented Sep 9, 2021

Might be good to make a PR just for the refactoring, and then a separate PR for the gradient.

@smartalecH smartalecH changed the title WIP: Subpixel smoothing gradients WIP: Meepgeom refactoring Sep 9, 2021
@smartalecH
Copy link
Collaborator Author

Here's a summary of the proposed changes:

  • Added new functions make_geom_epsilon(), init_libctl(), andset_materials_from_geom_epsilon().
  • Moved geom_epsilon class to meepgeom.hpp (previously local to the meepgeom.cpp only) so that I could pass around geom_epsilon objects in the overloaded set_materials_from_geom_epsilon() and make_geom_epsilon().
  • In order to move things to the meepgeom header file, I had to modify the name of the symmetric matrix routines so they wouldn't clash with mpb.cpp, which imports meepgeom.hpp and mpb.h, both of which have symmetric matrix functions (I couldn't get the namespaces to work properly).

All of my code compiles, but something weird is going on with the swig generated code -- particularly with the new set_materials_from_geom_epsilon() and make_geom_epsilon() functions.

Before I spend too much time debugging, can I get some feedback as to whether this is the best way to refactor things (I'm sure it's not...)?

@stevengj
Copy link
Collaborator

This looks fine.

@smartalecH
Copy link
Collaborator Author

smartalecH commented Sep 30, 2021

Almost all of the tests are passing now.

There is an issue with symmetric prism objects and subpixel smoothing. For example, two of the python tests (test_mode_coeffs.py and test_bend_flux.py) use a prism object to draw a simple, straight waveguide. Ideally, the waveguide should look like this:

image

Instead, it looks like this:

image

Notice that x>0 looks different than x<0. In fact, it seems that subpixel smoothing is not working for x<0 (for this particular prism object). If I turn subpixel smoothing off, I get:

image

So we know the issue is with subpixel smoothing and Prism objects. But not all prism objects, because the other prism object in the tests (a simple bend) looks fine Actually, this prism object looks off too (again, when x<0):

image

Any thoughts?
(Also, not sure why it won't compile using CI... I'll look into that after I debug this subpixel smoothing issue)

@codecov-commenter
Copy link

Codecov Report

Merging #1754 (fbd795f) into master (d41b0a4) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1754      +/-   ##
==========================================
+ Coverage   74.41%   74.43%   +0.02%     
==========================================
  Files          13       13              
  Lines        4581     4589       +8     
==========================================
+ Hits         3409     3416       +7     
- Misses       1172     1173       +1     
Impacted Files Coverage Δ
python/simulation.py 76.72% <100.00%> (-0.06%) ⬇️
python/adjoint/objective.py 91.76% <0.00%> (+0.85%) ⬆️

@smartalecH smartalecH marked this pull request as ready for review September 30, 2021 21:26
@smartalecH
Copy link
Collaborator Author

Oof seems like the above issue was due to an outdated version of libctl on my machine.

Should be good for review/merging now.

@stevengj stevengj merged commit fa23466 into NanoComp:master Oct 1, 2021
@stevengj
Copy link
Collaborator

stevengj commented Oct 1, 2021

LGTM.

@smartalecH smartalecH deleted the sub_gradient branch October 1, 2021 14:29
mawc2019 pushed a commit to mawc2019/meep that referenced this pull request Nov 3, 2021
* init

* get compile working

* get compilation working

* cleanup

* more cleanup

* fix failing c tests

Co-authored-by: Alec Hammond <[email protected]>
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

Successfully merging this pull request may close these issues.

subpixel smoothing gradients
3 participants