From 2649063bdba520495825e9e32973afadf6581f02 Mon Sep 17 00:00:00 2001 From: Jean Senellart Date: Mon, 9 May 2022 13:37:14 +0200 Subject: [PATCH 1/9] identified 2 source of memory access errors --- camfr/math/calculus/croot/patterson_z_n.cpp | 16 +++++++++++----- camfr/stack.cpp | 5 ++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/camfr/math/calculus/croot/patterson_z_n.cpp b/camfr/math/calculus/croot/patterson_z_n.cpp index 04b2c2b1..6d740928 100644 --- a/camfr/math/calculus/croot/patterson_z_n.cpp +++ b/camfr/math/calculus/croot/patterson_z_n.cpp @@ -43,6 +43,7 @@ vector patterson_z_n(ComplexFunction& f, { vector result; + // TODO: - missing abs_error definition for (unsigned int i=0; i<=M; i++) result.push_back(0.0); @@ -268,6 +269,7 @@ vector patterson_quad_z_n_sub bool converged = true; for (unsigned int i=0; i= abs_error size if ( abs(abs_error[i]) > abs(result_estimate[i] * eps) ) converged = false; @@ -276,11 +278,13 @@ vector patterson_quad_z_n_sub // Do adaptive subdivision of interval. + // CHECK: M result size can be < M - so we need to call following functions with updated M otherwise it will + // crash when estimating abs_error vector result1 - = patterson_quad_z_n_sub(f, a, (a+b)/2., M,eps,mu,result_estimate,max_k); + = patterson_quad_z_n_sub(f, a, (a+b)/2., result.size()-1,eps,mu,result_estimate,max_k); vector result2 - = patterson_quad_z_n_sub(f, (a+b)/2., b, M,eps,mu,result_estimate,max_k); + = patterson_quad_z_n_sub(f, (a+b)/2., b, result.size()-1,eps,mu,result_estimate,max_k); unsigned int new_M = (result1.size()>result2.size()) ? result2.size() : result1.size(); @@ -314,12 +318,14 @@ vector patterson_quad_z_n(ComplexFunction& f, return result; // Do adaptive subdivision of interval - + + // CHECK: M result size can be < M - so we need to call following functions with updated M otherwise it will + // crash when estimating abs_error vector result1 - = patterson_quad_z_n_sub(f, a, (a+b)/2., M, eps, mu, result, max_k); + = patterson_quad_z_n_sub(f, a, (a+b)/2., result.size()-1, eps, mu, result, max_k); vector result2 - = patterson_quad_z_n_sub(f, (a+b)/2., b, M, eps, mu, result, max_k); + = patterson_quad_z_n_sub(f, (a+b)/2., b, result.size()-1, eps, mu, result, max_k); unsigned int new_M = (result1.size()>result2.size()) ? result2.size() : result1.size(); diff --git a/camfr/stack.cpp b/camfr/stack.cpp index 5c88e305..2e6fc84b 100644 --- a/camfr/stack.cpp +++ b/camfr/stack.cpp @@ -109,7 +109,10 @@ StackImpl::StackImpl(const Expression& e_, unsigned int no_of_periods_) } // Create chunks. - + + // CHECK: we can have e.get_size()==1 here -> access to e.get_term(i+1) will be out of bound + // I tried to set max boundary to get_size()-1 - but the absence of chunk seems to create problems + // downstream for (unsigned int i=0; i Date: Mon, 9 May 2022 14:50:56 +0200 Subject: [PATCH 2/9] fix memory access problem --- camfr/stack.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/camfr/stack.cpp b/camfr/stack.cpp index 2e6fc84b..8b2d37f5 100644 --- a/camfr/stack.cpp +++ b/camfr/stack.cpp @@ -110,18 +110,17 @@ StackImpl::StackImpl(const Expression& e_, unsigned int no_of_periods_) // Create chunks. - // CHECK: we can have e.get_size()==1 here -> access to e.get_term(i+1) will be out of bound - // I tried to set max boundary to get_size()-1 - but the absence of chunk seems to create problems - // downstream for (unsigned int i=0; iget_type() != WAVEGUIDE) - if ( (i+1 < e.get_size()) && (t2->get_type() == WAVEGUIDE) ) + if (t2 && (t2->get_type() == WAVEGUIDE) ) { Complex d = t2->get_d(); From a9083afd924e15a56e507bced183f3a4ff62b1a2 Mon Sep 17 00:00:00 2001 From: Jean Senellart Date: Mon, 9 May 2022 18:33:14 +0200 Subject: [PATCH 3/9] add missing boundary checks --- camfr/math/calculus/croot/allroots.cpp | 7 +++++-- camfr/math/calculus/polyroot/polyroot.cpp | 3 ++- camfr/util/cvector.cpp | 21 +++++++++++++++------ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/camfr/math/calculus/croot/allroots.cpp b/camfr/math/calculus/croot/allroots.cpp index cb052dd4..f229c825 100755 --- a/camfr/math/calculus/croot/allroots.cpp +++ b/camfr/math/calculus/croot/allroots.cpp @@ -64,7 +64,7 @@ vector roots_contour(const Contour& contour, // Check if all G's are zero. bool all_zeros = true; - for (unsigned int i=0; i<=2*N-1; i++) + for (unsigned int i=0; i 1e-10) all_zeros = false; @@ -83,7 +83,10 @@ vector roots_contour(const Contour& contour, A(i,j) = G[i+j-2]; for (int i=1; i<=N; i++) - B(i,1) = -G[N+i-1]; + // CHECK: something strange here, I added boundary checking - however logically we should start from where the + // previous loop ends with N+i-2 - but changing this does not work, so I am adding this boundary check + if (N+i-1!=G.size()) + B(i,1) = -G[N+i-1]; // Solve linear system and exploit the symmetry. Note: the fact that this // is a Hankel matrix is not used, since speedups would probably be minor diff --git a/camfr/math/calculus/polyroot/polyroot.cpp b/camfr/math/calculus/polyroot/polyroot.cpp index fe01da01..a0e5ee52 100644 --- a/camfr/math/calculus/polyroot/polyroot.cpp +++ b/camfr/math/calculus/polyroot/polyroot.cpp @@ -55,7 +55,6 @@ vector polyroot(const vector& coef) py_error("Warning: polyroot solver did not converge."); delete [] coef_r; delete [] coef_i; - delete [] root_r, delete [] root_i; // Return results. @@ -63,5 +62,7 @@ vector polyroot(const vector& coef) for (unsigned int i=0; i operator+(const vector& a, { vector result; - for (unsigned int i=0; i operator-(const vector& a, { vector result; - for (unsigned int i=0; i& operator-=( vector& a, From 22b329d9075510f2d79274dc418b01e1593cfb4e Mon Sep 17 00:00:00 2001 From: Jean Senellart Date: Mon, 9 May 2022 18:53:37 +0200 Subject: [PATCH 4/9] numpy API to create array changed --- camfr/camfr_wrap.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/camfr/camfr_wrap.cpp b/camfr/camfr_wrap.cpp index 37361684..10b66ee3 100644 --- a/camfr/camfr_wrap.cpp +++ b/camfr/camfr_wrap.cpp @@ -365,11 +365,12 @@ inline void free_tmp_interfaces(Waveguide& w) struct cVector_to_python { static PyObject* convert(const cVector& c) - { - int dim[1]; dim[0] = c.rows(); + { + npy_intp dim[1]; + dim[0] = c.rows(); - PyArrayObject* result - = (PyArrayObject*) PyArray_FromDims(1, dim, PyArray_CDOUBLE); + PyArrayObject* result + = (PyArrayObject*) PyArray_SimpleNew(1, dim, PyArray_CDOUBLE); for (int i=0; idata + i*result->strides[0]) = c(i+1); @@ -427,10 +428,10 @@ struct cMatrix_to_python { static PyObject* convert(const cMatrix& c) { - int dim[2]; dim[0] = c.rows(); dim[1] = c.columns(); + npy_intp dim[2]; dim[0] = c.rows(); dim[1] = c.columns(); PyArrayObject* result - = (PyArrayObject*) PyArray_FromDims(2, dim, PyArray_CDOUBLE); + = (PyArrayObject*) PyArray_SimpleNew(2, dim, PyArray_CDOUBLE); for (int i=0; i Date: Mon, 9 May 2022 18:56:45 +0200 Subject: [PATCH 5/9] fix missing boundary check --- camfr/primitives/slab/isoslab/slab.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/camfr/primitives/slab/isoslab/slab.cpp b/camfr/primitives/slab/isoslab/slab.cpp index d982804b..38f854b5 100755 --- a/camfr/primitives/slab/isoslab/slab.cpp +++ b/camfr/primitives/slab/isoslab/slab.cpp @@ -1489,7 +1489,7 @@ void Slab_M::build_modeset(vector& kt) if (i == 0) { - if (real(materials[i]->eps_mu()) > real(materials[i+1]->eps_mu())) + if (materials.size()>1 && real(materials[i]->eps_mu()) > real(materials[i+1]->eps_mu())) is_core = true; } else if (i == materials.size()-1) From 189c9a94cc0973a16a6d3c365f18bc61eb430343 Mon Sep 17 00:00:00 2001 From: Jean Senellart Date: Mon, 9 May 2022 19:02:06 +0200 Subject: [PATCH 6/9] fix compilation warnings --- camfr/math/calculus/croot/patterson_z_n.cpp | 5 ----- camfr/mode.h | 3 ++- camfr/primitives/blochsection/blochsection.cpp | 2 ++ camfr/stack.cpp | 5 +++-- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/camfr/math/calculus/croot/patterson_z_n.cpp b/camfr/math/calculus/croot/patterson_z_n.cpp index 6d740928..0d06e71e 100644 --- a/camfr/math/calculus/croot/patterson_z_n.cpp +++ b/camfr/math/calculus/croot/patterson_z_n.cpp @@ -269,7 +269,6 @@ vector patterson_quad_z_n_sub bool converged = true; for (unsigned int i=0; i= abs_error size if ( abs(abs_error[i]) > abs(result_estimate[i] * eps) ) converged = false; @@ -278,8 +277,6 @@ vector patterson_quad_z_n_sub // Do adaptive subdivision of interval. - // CHECK: M result size can be < M - so we need to call following functions with updated M otherwise it will - // crash when estimating abs_error vector result1 = patterson_quad_z_n_sub(f, a, (a+b)/2., result.size()-1,eps,mu,result_estimate,max_k); @@ -319,8 +316,6 @@ vector patterson_quad_z_n(ComplexFunction& f, // Do adaptive subdivision of interval - // CHECK: M result size can be < M - so we need to call following functions with updated M otherwise it will - // crash when estimating abs_error vector result1 = patterson_quad_z_n_sub(f, a, (a+b)/2., result.size()-1, eps, mu, result, max_k); diff --git a/camfr/mode.h b/camfr/mode.h index 4dfb42c3..ed745cb5 100644 --- a/camfr/mode.h +++ b/camfr/mode.h @@ -171,11 +171,12 @@ struct modesorter_BDM const double ra2 = real(a->get_kz() * a->get_kz()); const double rb2 = real(b->get_kz() * b->get_kz()); - if ( (ra2 < 0) && (rb2 < 0) ) + if ( (ra2 < 0) && (rb2 < 0) ) { if ( (a->pol != TE) && (a->pol != TM) ) return ( ra2 < rb2 ); else return ( ra2 > rb2 ); + } if ( (ra2 > 0) && (rb2 > 0) ) return ( ra2 > rb2 ); diff --git a/camfr/primitives/blochsection/blochsection.cpp b/camfr/primitives/blochsection/blochsection.cpp index bc43c2a7..4145a33a 100644 --- a/camfr/primitives/blochsection/blochsection.cpp +++ b/camfr/primitives/blochsection/blochsection.cpp @@ -1287,6 +1287,8 @@ int UniformBlochSection::order(Polarisation pol, int Mx, int My) const if ( (m->pol == pol) && (m->get_Mx() == Mx) && (m->get_My() == My) ) return i; } + // CHECK - what should be default return? + return 0; } diff --git a/camfr/stack.cpp b/camfr/stack.cpp index 8b2d37f5..a83750e8 100644 --- a/camfr/stack.cpp +++ b/camfr/stack.cpp @@ -119,7 +119,7 @@ StackImpl::StackImpl(const Expression& e_, unsigned int no_of_periods_) // No waveguide. - if (t1->get_type() != WAVEGUIDE) + if (t1->get_type() != WAVEGUIDE) { if (t2 && (t2->get_type() == WAVEGUIDE) ) { Complex d = t2->get_d(); @@ -141,7 +141,8 @@ StackImpl::StackImpl(const Expression& e_, unsigned int no_of_periods_) } else chunks.push_back(Chunk(t1->get_sc(), 0.0)); - + } + // Waveguide. if (t1->get_type() == WAVEGUIDE) From e3749427bb780a719a172dcb25b44229f0009b9c Mon Sep 17 00:00:00 2001 From: Jean Senellart Date: Mon, 9 May 2022 22:37:11 +0200 Subject: [PATCH 7/9] updated macos installation instructions --- INSTALL.MacOSX | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/INSTALL.MacOSX b/INSTALL.MacOSX index cbf8a014..8e36dd86 100644 --- a/INSTALL.MacOSX +++ b/INSTALL.MacOSX @@ -1,5 +1,33 @@ COMPILING AND INSTALLING +The following instructions apply for use of package installer `homebrew`. Legacy instructions below were also showing +use of MacPorts for older version of python and it has not been tested + +# Installation with `homebrew` + +Install brew (https://docs.brew.sh/Installation) and Xcode ((from the App Store or from the apple developer website), agree to the Xcode license and install the Xcode command-line tools. + +Install the following brew package: +$ brew install texlive +$ brew install boost boost-python3 +$ brew install gcc lapack blitz scons + +Use pyenv (https://github.com/pyenv/pyenv) to install your favorite python 3 version. + +install CAMFR python requirements: + +$ pip install requirements.txt + +$ cp machine_cfg.py.MacOSX-brew machine_cfg.py + +check paths in machine_cfg.py - and run installation + +$ pip install . + +# Installation with MacPorts - and python 2.7 + +(this won't work on Apple M1 where python 2.7 is deprecated) + For the compilation of CAMFR on MacOSX, it is relatively straightforward to use MacPorts to install all dependencies, although with the drawback that the built-in Mac OS installation of Python will be superseded by the MacPorts python installation. The following instructions will install a new Python 2.7 interpreter, alongside your built-in MacOS Python installation. Be sure to used the correct python shell when running the CAMFR install script, and note that CAMFR will only be installed to this particular python interpreter unless you manually copy the module into another `site-packages` folder. From 520c1c6960cc7979c0e860b71490a08f2e9bad55 Mon Sep 17 00:00:00 2001 From: Jean Senellart Date: Wed, 11 May 2022 10:20:21 +0200 Subject: [PATCH 8/9] check on clean environment and updated installation procedure --- machine_cfg.py.MacOSX-brew | 8 +++++++- requirements.txt | 1 - 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/machine_cfg.py.MacOSX-brew b/machine_cfg.py.MacOSX-brew index 2dea44c2..9881211f 100644 --- a/machine_cfg.py.MacOSX-brew +++ b/machine_cfg.py.MacOSX-brew @@ -8,8 +8,14 @@ # brew install boost-python3 # brew install texlive +# make sure to have tcl-tk installed (brew install tcl-tk) before installing python (important because CAMFR has some built-in dependencies with tkinter) # define python version with pyenv and define virtualenv with venv +# finally update the variable PATHTOPYENVPYTHONVERSION/.pyenv..., with path to your pyenv python version (for Python.h include) and .venv version (for numpy includes) /PATHTOVENV/venv/lib/python3.9/... + +# check compilation with "make" +# then you can install with pip install . + # # is building on ARM mac book pro Monterey 12.1 @@ -45,7 +51,7 @@ flags = fflags + " -ftemplate-depth-60" include_dirs = ["/opt/local/include", "/opt/homebrew/Cellar//boost/1.78.0_1/include/", "/opt/homebrew/Cellar/blitz/1.0.2/include/", - "~/.pyenv/versions/3.9.11/include/python3.9", "venv/lib/python3.9/site-packages"] + "/PATHTOPYENVPYTHONVERSION/.pyenv/versions/3.9.11/include/python3.9", "/PATHTOVENV/venv/lib/python3.9/site-packages"] library_dirs = [ "/opt/homebrew/Cellar//boost/1.78.0_1/lib", "/opt/homebrew/Cellar/boost-python3/1.78.0/lib", "/opt/homebrew/Cellar/blitz/1.0.2/lib","/opt//homebrew/Cellar/gcc/11.3.0/lib/gcc/11/" diff --git a/requirements.txt b/requirements.txt index ed2733f9..06a4c992 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,6 @@ Image==1.5.33 ImageFilter==0.1.0 matplotlib==3.5.2 MLab==1.1.4 -Numeric==24.2 numpy==1.22.3 Pillow==9.1.0 pymat==0.0.2 From e340dcb9fd912343b78e3bd0621600647f9cfd25 Mon Sep 17 00:00:00 2001 From: Jean Senellart Date: Tue, 21 Mar 2023 12:05:13 +0100 Subject: [PATCH 9/9] fix #23: object keeps a reference to local variable --- camfr/waveguide.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/camfr/waveguide.h b/camfr/waveguide.h index 45d2c859..aec52214 100644 --- a/camfr/waveguide.h +++ b/camfr/waveguide.h @@ -61,10 +61,12 @@ class Waveguide { public: - Waveguide(bool uniform_=false, Material* core_=NULL) - : uniform(uniform_), core(core_) {} - - virtual ~Waveguide() {interface_cache.deregister(this);} // Important! + Waveguide(bool uniform_=false, Material* core_=NULL): uniform(uniform_), core(new Material(*core_)) {} + + virtual ~Waveguide() { + delete core; + interface_cache.deregister(this); // Important! + } bool is_uniform() const {return uniform;} Material* get_core() const {return core;}