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

additional improvements for MaterialGrid #1512

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

oskooi
Copy link
Collaborator

@oskooi oskooi commented Feb 25, 2021

Additional improvements to MaterialGrid following @stevengj's two unresolved comments in #1508:

  • rename design_parameters to weights
  • rename U_SUM to U_MEAN

Also includes more fixes and tweaks for the docstrings.

Additionally, this PR fixes a bug in libpympb/mpb.h related to the name of an imported header file that was discovered during the course of development:

#include "meepgeom.hpp"

The preprocessor for GCC was loading meepgeom.hpp found in the system directory /usr/local/include/meep rather than the file in the source tree (from the meep/src subdirectory). As a result, this was causing an error during make because design_parameters had been renamed to weights in meepgeom.cpp but not in meepgeom.hpp. The fix involved changing this line to:

#include "../src/meepgeom.hpp"

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@stevengj stevengj merged commit 5c092dd into NanoComp:master Mar 3, 2021
@oskooi
Copy link
Collaborator Author

oskooi commented Mar 3, 2021

Configure command:

./configure --enable-maintainer-mode --enable-shared --with-mpi PYTHON=python3.5 LDFLAGS=-L/usr/lib/x86_64-linux-gnu/hdf5/openmpi -L/usr/local/lib -Wl,-rpath,/usr/lib/x86_64-linux-gnu/hdf5/openmpi:/usr/local/lib CPPFLAGS=-I/usr/include/hdf5/openmpi -I/usr/local/include

@oskooi
Copy link
Collaborator Author

oskooi commented Mar 3, 2021

RPATH_FLAGS="-Wl,-rpath,/usr/lib/x86_64-linux-gnu/hdf5/openmpi:/usr/local/lib"
MY_LDFLAGS="-L/usr/lib/x86_64-linux-gnu/hdf5/openmpi -L/usr/local/lib ${RPATH_FLAGS}"
MY_CPPFLAGS="-I/usr/include/hdf5/openmpi -I/usr/local/include"

sh autogen.sh --enable-shared --with-mpi PYTHON=python3.5 LDFLAGS="${MY_LDFLAGS}" CPPFLAGS="${MY_CPPFLAGS}"
make

@oskooi oskooi deleted the matgrid_improvements branch March 3, 2021 02:16
@stevengj
Copy link
Collaborator

stevengj commented Mar 3, 2021

meepgeom.hpp is installed in /usr/local/include/meep, not /usr/local/include, so -I/usr/local/include won't cause it to be found by #include "meepgeom.h"

Furthermore, gcc searches the -I paths in left to right order. This means that the -I$(top_srcdir)/src specified in AM_CPPFLAGS by libpympb/Makefile.am will take precedence over any -I flags you specify in CPPFLAGS, because CPPFLAGS appears after AM_CPPFLAGS in automake's flag ordering.

So, I don't think that change is correct.

bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants