Fix performance issues in ModeSolver.find_k #644
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes two issues that made python's
mpb.ModeSolver.find_k
about 7x slower than the corresponding Schemefind-k
.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 thedefault_material
andgeometry
can be modified between runs. Scheme uses global variables for modifiable parameters, but in Python we have to pass the information from theModeSolver
Python object to themode_solver
C++ instance.For readability I had split an
if
clause into multiple lines inpython/geom.py:find_root_deriv
. However, in Scheme the final condition (which I calledcond
) was only evaluated if the first 2and
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