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

Fix performance issues in ModeSolver.find_k #644

Merged
merged 6 commits into from
Dec 31, 2018
Merged

Fix performance issues in ModeSolver.find_k #644

merged 6 commits into from
Dec 31, 2018

Conversation

ChristopherHogan
Copy link
Contributor

@ChristopherHogan ChristopherHogan commented Dec 21, 2018

This fixes two issues that made python's mpb.ModeSolver.find_k about 7x slower than the corresponding Scheme find-k.

  1. Scheme reuses fields between runs when it detects that a suitable maxwell_data is already available. This PR adds the same functionality for Python. However, only the default_material and geometry can be modified between runs. Scheme uses global variables for modifiable parameters, but in Python we have to pass the information from the ModeSolver Python object to the mode_solver C++ instance.

  2. For readability I had split an if clause into multiple lines in python/geom.py:find_root_deriv. However, in Scheme the final condition (which I called cond) was only evaluated if the first 2 and conditions were true, whereas in Python they were evaluated every time, leading to extra unnecessary work.

Python's find_k is now about 20% faster than Scheme's (although Python doesn't output epsilon by default like Scheme does).

I'll add a documentation PR to MPB explaining how to get the performance gains of reusing fields.

@stevengj @oskooi

@ChristopherHogan
Copy link
Contributor Author

Added property accessors that allow changes to the Python ModeSolver to propagate to the C++ mode_solver.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 82.378% when pulling 06c88ab on ChristopherHogan:chogan/mpb_reuse_fields into 5be326d on stevengj:master.

@stevengj stevengj merged commit b443ee4 into NanoComp:master Dec 31, 2018
@ChristopherHogan ChristopherHogan deleted the chogan/mpb_reuse_fields branch January 2, 2019 16:40
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
* Reuse fields if able

* Allow reusing ModeSolver objects

* Don't prematurely evaluate rootfun

* Add property accessor for ModeSolver's C++ mode_solver

* Fix resolution and lattice accessors

* Adjust grid_size when resolution or geometry_lattice are changed
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.

3 participants