Fix memory leaks in SWIG wrappers. #1826
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1826 +/- ##
==========================================
- Coverage 74.43% 73.53% -0.90%
==========================================
Files 13 17 +4
Lines 4589 4879 +290
==========================================
+ Hits 3416 3588 +172
- Misses 1173 1291 +118 |
stevengj
reviewed
Nov 15, 2021
python/meep.i
Outdated
| @@ -1405,6 +1405,7 @@ void _get_gradient(PyObject *grad, PyObject *fields_a, PyObject *fields_f, PyObj | |||
| if ($1.num_items != 0) { | |||
| for (int i = 0; i < $1.num_items; i++) { | |||
| material_free($1.items[i]); | |||
| delete $1.items[i]; | |||
There was a problem hiding this comment.
Maybe it would be better to put this in material_free?
There was a problem hiding this comment.
We could - but then it might break the code of anybody using the C++ api directly.
I dont know how much we worry about that.
There was a problem hiding this comment.
I'm not so worried about that.
There was a problem hiding this comment.
I pushed a patch to move delete into material_free. It looks like there was another memory leak in set_default_material that this fixes.
There was a problem hiding this comment.
Thanks for making the change. I tested this internally and everything looks good.
Please go ahead and merge this change.
mochen4
pushed a commit
to mochen4/meep
that referenced
this pull request
Feb 16, 2022
* Fix memory leaks in SWIG wrappers. * move delete into material_free Co-authored-by: Steven G. Johnson <stevenj@alum.mit.edu>
mochen4
pushed a commit
to mochen4/meep
that referenced
this pull request
Feb 16, 2022
* Fix memory leaks in SWIG wrappers. * move delete into material_free Co-authored-by: Steven G. Johnson <stevenj@alum.mit.edu>
stevengj
added a commit
that referenced
this pull request
Mar 10, 2022
* first draft of loop_in_chunks using grid_vol calculations * try to only use owned points in loop_in_chunks * don't unpad if !use_symmetry * stop vscode from complaining about comments * rm hack to loop over centered grid * fix and example * almost fix * before fix * one more loop over num_chunk * include set * different approach * minor * include climits * include climits * fix retval * clean up * using flipped * add missing changes * fix bug * fix pml * Fix memory leaks in SWIG wrappers. (#1826) * Fix memory leaks in SWIG wrappers. * move delete into material_free Co-authored-by: Steven G. Johnson <stevenj@alum.mit.edu> * Fix adjoint gradient with conductivities (#1830) * damp_fix * increase run time Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local> * plot geometry for dispersive materials without initializing structure object (#1827) * plot geometry without initializing structure class * update docstrings * rotate epsilon grid by 90 degrees to properly orient axes * add support for dispersive ε * update markdown pages from docstrings * make frequency and resolution parameters of plot2D dictionary keys of eps_parameters * reinstate frequency parameter of plot2D and add warning that it is deprecated * fix order of frequency check * unit test for `get_epsilon_grid` (#1835) * unit test for get_epsilon_grid * fix * limit test points to homogeneous regions (i.e. no material interfaces) and away from potential chunk boundaries * support for single-precision floating point for fields array functions (#1833) * switch dft-related functions to using realnum from double * more fixes * more type conversions from double to realnum * adjust check tolerance of tests/integrate.cpp based on floating-point precision * more fixes * rebase from master from fix merge conflicts * slight adjustment to tolerances in unit tests and update docs * remove type check in test_adjoint_solver.py * revert return types of integration functions to double * revert return type of process_dft_component to double * cleanup * Fix memory leaks (#1839) * Fix memory leaks * Add kkg to authors list * Fix for Issue #1834 (#1840) * Fix memory leaks * Add kkg to authors list * Expose set_default_material and use it in libpympb/pympb.cpp * use unique_ptr (C++11) instead of make_unique (C++14) (#1844) * Use None instead of empty list in constructors (#1846) * use None * minor fix Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local> * Define what happens when `β=∞` and `u=η` (#1842) * define what happens when beta=inf and u=0.5 * use eta not 0.5 * Update src/meepgeom.cpp Co-authored-by: Steven G. Johnson <stevenj@mit.edu> * fix for arrays (#1845) * minor improvements to docs (#1848) * update homebrew instructions for hdf5 and openblas (fixes #1850) * recommend python3 on macos * silence compiler warnings * whoops, missing commit * tests need scipy and autograd * missing sudo * parameterized package is also used for tests * h5py and jax on mac * note on autogen.sh for git clone * xcode installation shortcut * bug fix for get_epsilon_point and cell boundary in parallel simulation (#1849) * bug fix for get_epsilon_point and cell boundary in parallel simulation * check for six digits in test_material_grid.py because of single precision * unit test for conductivity (#1857) * unit test for conductivity * describe in the docs how to model the attenutation coefficient using conductivity * Update python/tests/test_conductivity.py Co-authored-by: Steven G. Johnson <stevenj@mit.edu> * Fix the failure message for absorber 1D test (#1859) * add missing fixed field phase to MPB unit test (#1860) * fix memory leak in array-slice-ll.cpp (#1865) * fix memory leak in array-slice-ll.cpp * reinstate line break * fix memory leak in cyl-ellipsoid-ll.cpp (#1866) * level parameter for contour plot calls epsilon data (#1869) * fix heap buffer overflow error for update E from D in cylindrical coordinates (#1871) * Add cylindrical coordinates support for `plot2d` (#1873) * add visualization support for plot2d * bug fix with cartesian plotting * fix near-field monitor position in cylindrical coordinate tutorial (#1874) * fix memory leaks in structure and fields load during checkpointing (#1872) * fix memory leaks in structure and fields load during checkpointing * delete the chi1inv and fields array if it exists and reallocate * in unit test, set gaussian source cutoff to 0 due to off-by-1 timestep counter bug * remove cutoff=0 from unit tests * lazily allocate H only if B is not NULL * allocate fields array for H in PML region * fix two memory leaks in geom_epsilon class (#1877) * fix two memory leaks in geom_epsilon class * delete global variable default_material at the end of unit tests * add unset_default_material function to class meep_geom * include ring-ll.cpp in C++ unit tests (#1878) * include ring-ll.cpp in C++ unit tests * only validate Harminv modes with error below some threshold * fix and example * first draft of loop_in_chunks using grid_vol calculations * try to only use owned points in loop_in_chunks * don't unpad if !use_symmetry * stop vscode from complaining about comments * rm hack to loop over centered grid * almost fix * before fix * one more loop over num_chunk * include set * different approach * minor * include climits * include climits * fix retval * clean up * using flipped * add missing changes * fix bug * fix pml * Fix memory leaks in SWIG wrappers. (#1826) * Fix memory leaks in SWIG wrappers. * move delete into material_free Co-authored-by: Steven G. Johnson <stevenj@alum.mit.edu> * Fix adjoint gradient with conductivities (#1830) * damp_fix * increase run time Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local> * plot geometry for dispersive materials without initializing structure object (#1827) * plot geometry without initializing structure class * update docstrings * rotate epsilon grid by 90 degrees to properly orient axes * add support for dispersive ε * update markdown pages from docstrings * make frequency and resolution parameters of plot2D dictionary keys of eps_parameters * reinstate frequency parameter of plot2D and add warning that it is deprecated * fix order of frequency check * unit test for `get_epsilon_grid` (#1835) * unit test for get_epsilon_grid * fix * limit test points to homogeneous regions (i.e. no material interfaces) and away from potential chunk boundaries * support for single-precision floating point for fields array functions (#1833) * switch dft-related functions to using realnum from double * more fixes * more type conversions from double to realnum * adjust check tolerance of tests/integrate.cpp based on floating-point precision * more fixes * rebase from master from fix merge conflicts * slight adjustment to tolerances in unit tests and update docs * remove type check in test_adjoint_solver.py * revert return types of integration functions to double * revert return type of process_dft_component to double * cleanup * Fix memory leaks (#1839) * Fix memory leaks * Add kkg to authors list * Fix for Issue #1834 (#1840) * Fix memory leaks * Add kkg to authors list * Expose set_default_material and use it in libpympb/pympb.cpp * use unique_ptr (C++11) instead of make_unique (C++14) (#1844) * Use None instead of empty list in constructors (#1846) * use None * minor fix Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local> * Define what happens when `β=∞` and `u=η` (#1842) * define what happens when beta=inf and u=0.5 * use eta not 0.5 * Update src/meepgeom.cpp Co-authored-by: Steven G. Johnson <stevenj@mit.edu> * fix for arrays (#1845) * minor improvements to docs (#1848) * update homebrew instructions for hdf5 and openblas (fixes #1850) * recommend python3 on macos * silence compiler warnings * whoops, missing commit * tests need scipy and autograd * missing sudo * parameterized package is also used for tests * h5py and jax on mac * note on autogen.sh for git clone * xcode installation shortcut * bug fix for get_epsilon_point and cell boundary in parallel simulation (#1849) * bug fix for get_epsilon_point and cell boundary in parallel simulation * check for six digits in test_material_grid.py because of single precision * unit test for conductivity (#1857) * unit test for conductivity * describe in the docs how to model the attenutation coefficient using conductivity * Update python/tests/test_conductivity.py Co-authored-by: Steven G. Johnson <stevenj@mit.edu> * Fix the failure message for absorber 1D test (#1859) * add missing fixed field phase to MPB unit test (#1860) * fix memory leak in array-slice-ll.cpp (#1865) * fix memory leak in array-slice-ll.cpp * reinstate line break * fix memory leak in cyl-ellipsoid-ll.cpp (#1866) * level parameter for contour plot calls epsilon data (#1869) * fix heap buffer overflow error for update E from D in cylindrical coordinates (#1871) * Add cylindrical coordinates support for `plot2d` (#1873) * add visualization support for plot2d * bug fix with cartesian plotting * fix near-field monitor position in cylindrical coordinate tutorial (#1874) * fix memory leaks in structure and fields load during checkpointing (#1872) * fix memory leaks in structure and fields load during checkpointing * delete the chi1inv and fields array if it exists and reallocate * in unit test, set gaussian source cutoff to 0 due to off-by-1 timestep counter bug * remove cutoff=0 from unit tests * lazily allocate H only if B is not NULL * allocate fields array for H in PML region * fix two memory leaks in geom_epsilon class (#1877) * fix two memory leaks in geom_epsilon class * delete global variable default_material at the end of unit tests * add unset_default_material function to class meep_geom * include ring-ll.cpp in C++ unit tests (#1878) * include ring-ll.cpp in C++ unit tests * only validate Harminv modes with error below some threshold * fix and example * first draft of loop_in_chunks using grid_vol calculations * try to only use owned points in loop_in_chunks * don't unpad if !use_symmetry * stop vscode from complaining about comments * rm hack to loop over centered grid * almost fix * before fix * one more loop over num_chunk * include set * different approach * minor * include climits * include climits * fix retval * clean up * using flipped * add missing changes * fix bug * fix pml * Fix adjoint gradient with conductivities (#1830) * damp_fix * increase run time Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local> * plot geometry for dispersive materials without initializing structure object (#1827) * plot geometry without initializing structure class * update docstrings * rotate epsilon grid by 90 degrees to properly orient axes * add support for dispersive ε * update markdown pages from docstrings * make frequency and resolution parameters of plot2D dictionary keys of eps_parameters * reinstate frequency parameter of plot2D and add warning that it is deprecated * fix order of frequency check * support for single-precision floating point for fields array functions (#1833) * switch dft-related functions to using realnum from double * more fixes * more type conversions from double to realnum * adjust check tolerance of tests/integrate.cpp based on floating-point precision * more fixes * rebase from master from fix merge conflicts * slight adjustment to tolerances in unit tests and update docs * remove type check in test_adjoint_solver.py * revert return types of integration functions to double * revert return type of process_dft_component to double * cleanup * Fix memory leaks (#1839) * Fix memory leaks * Add kkg to authors list * Fix for Issue #1834 (#1840) * Fix memory leaks * Add kkg to authors list * Expose set_default_material and use it in libpympb/pympb.cpp * use unique_ptr (C++11) instead of make_unique (C++14) (#1844) * update homebrew instructions for hdf5 and openblas (fixes #1850) * rm hack to loop over centered grid * almost fix * one more loop over num_chunk * different approach * include climits * include climits * clean up * using flipped * Revert "fix pml" This reverts commit 9ed741e. * fix rebase * fix rebase * fix rebase * fix rebase * fix rebase * fix error * increase tol * cleanup * cleanup Co-authored-by: Steven G. Johnson <stevenj@alum.mit.edu> Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local> Co-authored-by: Mo Chen <mochen@dhcp-10-29-91-247.dyn.MIT.EDU> Co-authored-by: Krishna Gadepalli <34969407+kkg4theweb@users.noreply.github.com> Co-authored-by: Ardavan Oskooi <ardavano@google.com> Co-authored-by: Steven G. Johnson <stevenj@mit.edu> Co-authored-by: Alec Hammond <alec.m.hammond@gmail.com> Co-authored-by: Andreas Hoenselaar <ahoens@google.com> Co-authored-by: simbilod <46427609+simbilod@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes memory leaks in the SWIG wrappers for meep::make_output_directory and the automatic freeing of material_type_list