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.

Sign up for GitHub

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

Jump to bottom

Fix images in MPB data analysis tutorial #695

Merged
merged 4 commits into from
Feb 5, 2019
Merged

Fix images in MPB data analysis tutorial #695

merged 4 commits into from
Feb 5, 2019

Conversation

ChristopherHogan
Copy link
Contributor

Fixes #694. I'm not sure exactly what happened here, but #644 broke mpb_data_analysis.py. It looks like one ms import was overwriting the other somehow. Importing them in separate functions fixes it.
@stevengj @oskooi

@ChristopherHogan
Copy link
Contributor Author

Oh, it's because geometry_lattice is a libctl global variable. Importing one ModeSolver after the other overwrote that parameter.

@JohnWeiner
Copy link

JohnWeiner commented Feb 3, 2019 via email

@JohnWeiner
Copy link

JohnWeiner commented Feb 3, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

Is there some way that I can replace my existing mpb_data_analysis.py with the revised version that should now work?

In general, once a PR is merged into the master branch the change will trigger a rebuild of the nightly conda package, which should be on the dev label of the chogan channel and available for install within 24 ours of the merge.

Alternatively, if you want to experiment before the PR is merged, you can check out the PR branch and build from source.

@ChristopherHogan
Copy link
Contributor Author

@JohnWeiner, did you build from source or install a conda package? Can you run the following command to show the pymeep version?

$ python -c 'import meep; print(meep.__version__)'

It should work with the latest master branch.

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

Yes, that's a bit outdated. You can try updating the pymeep package like this

$ conda update -c chogan/label/dev -c chogan -c conda-forge pymeep # or pymeep-parallel

However, I've found that conda update can often break an environment so I usually prefer to create a new environment (since they're cheap), keeping the old one in case you need to roll back.

$ conda create -n mp1.7 -c chogan/label/dev -c chogan -c conda-forge pymeep
$ conda activate mp1.7

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

ChristopherHogan commented Feb 4, 2019 •
edited
Loading

The general command is

$ conda activate <environment name>

The name of the environment is whatever you pass to the -n flag in the conda create command. I think the meep docs still say

$ source activate <environment name>

which accomplishes the same thing, but source activate is deprecated and conda activate is now preferred.

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

Is that after commenting out the from mpb_diamond import ms as d_ms and diamond() lines (since this fix isn't applied yet to the nightly builds)?

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

I can reproduce this. MPB seems to be linking with a system libBLAS.dylib instead of the conda installed libopenblas.dylib, even though otool -L reports that libmpb.dylib is linked with libopenblas.dylib.

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

Right, that behavior is what this PR fixes. If you move the imports to their respective functions (the changes in this PR) you will see the correct images.

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

ChristopherHogan commented Feb 4, 2019 •
edited
Loading

This conversation is happening in a thread for a pull request, the diff of which is at https://github.com/NanoComp/meep/pull/695/files. Those changes fix the issue you see on Linux.

@ChristopherHogan
Copy link
Contributor Author

The Mac issue is different though. It has to do with the way the conda packages are built. I'm investigating that now.

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

Is there a better way to incorporate the PR into the py file?

That's the process I explained in the comment here: #695 (comment)

Instructions on checking out a local copy of a PR are here: https://help.github.com/articles/checking-out-pull-requests-locally/

@JohnWeiner
Copy link

JohnWeiner commented Feb 4, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

ChristopherHogan commented Feb 4, 2019 •
edited
Loading

The Mac issue is due to the matplotlib package and the fix is twofold.

  1. Import meep before importing matplotlib
  2. Set the default backend to TkAgg via the matplotlibrc file:
$ mkdir -p ~/.matplotlib
$ echo "backend: TkAgg" > ~/.matplotlib/matplotlibrc

@JohnWeiner
Copy link

JohnWeiner commented Feb 5, 2019 •
edited by ChristopherHogan
Loading

I incorporated the changes you propose in mpb_data_analysis.py: to wit,

import meep before matplotlib,
$ mkdir -p ~/.matplotlib $ echo "backend: TkAgg" > ~/.matplotlib/matplotlibrc,

and

def tri_rods():
# Import the ModeSolver defined in the mpb_tri_rods.py example
from mpb_tri_rods import ms as tr_ms
efields = []

# Band function to collect the efields

def get_efields(tr_ms, band):

def diamond():
# Import the ModeSolver from the mpb_diamond.py example
from mpb_diamond import ms as d_ms

  dpwr = []

def get_dpwr(ms, band):

However the mpb_data_analysis.py script still does not run. I get this puzzling error when I execute python command on the file:
$ python ./miniconda3//pkgs/pymeep-1.7.1.dev-py36_nomkl_25/info/test/python/examples/mpb_data_analysis.py
Traceback (most recent call last):
File "./miniconda3//pkgs/pymeep-1.7.1.dev-py36_nomkl_25/info/test/python/examples/mpb_data_analysis.py", line 28, in
tr_ms.run_tm(mpb.output_at_kpoint(mp.Vector3(1 / -3, 1 / 3), mpb.fix_efield_phase,
NameError: name 'tr_ms' is not defined.

It is as if the subroutine,

def tri_rods():
# Import the ModeSolver defined in the mpb_tri_rods.py example
from mpb_tri_rods import ms as tr_ms
efields = []

is not read or something.

Here is what the tri_rods part of the mpb_data_analysis.py file looks like after the modifications:

from __future__ import division

import os
import sys
import numpy as np
import meep as mp
import matplotlib.pyplot as plt
from meep import mpb

examples_dir = os.path.realpath(os.path.dirname(__file__))
sys.path.insert(0, examples_dir)


def tri_rods():
    # Import the ModeSolver defined in the mpb_tri_rods.py example
    from mpb_tri_rods import ms as tr_ms
    efields = []

    # Band function to collect the efields
def get_efields(tr_ms, band):
    efields.append(tr_ms.get_efield(band))

tr_ms.run_tm(mpb.output_at_kpoint(mp.Vector3(1 / -3, 1 / 3), mpb.fix_efield_phase,
                                      get_efields))

    # Create an MPBData instance to transform the efields
md = mpb.MPBData(rectify=True, resolution=32, periods=3)

converted = []
for f in efields:
    # Get just the z component of the efields
    f = f[..., 0, 2]
    converted.append(md.convert(f))

tr_ms.run_te()

eps = tr_ms.get_epsilon()
plt.imshow(eps.T, interpolation='spline36', cmap='binary')
plt.axis('off')
plt.show()

md = mpb.MPBData(rectify=True, resolution=32, periods=3)
rectangular_data = md.convert(eps)
plt.imshow(rectangular_data.T, interpolation='spline36', cmap='binary')
plt.axis('off')
plt.show()

for i, f in enumerate(converted):
    plt.subplot(331 + i)
    plt.contour(rectangular_data.T, cmap='binary')
    plt.imshow(np.real(f).T, interpolation='spline36', cmap='RdBu', alpha=0.9)
    plt.axis('off')
 
plt.show()

@ChristopherHogan
Copy link
Contributor Author

I'm not sure if this is just a cut-and-paste issue, but the tri_rods code you show has some indentation issues that would break things. Can you make sure the indentation is exactly the same as https://github.com/NanoComp/meep/blob/master/python/examples/mpb_data_analysis.py?

@JohnWeiner
Copy link

JohnWeiner commented Feb 5, 2019 via email

@ChristopherHogan
Copy link
Contributor Author

Great!

@stevengj stevengj merged commit 94d2168 into NanoComp:master Feb 5, 2019
@ChristopherHogan ChristopherHogan deleted the chogan/issue_694 branch February 8, 2019 14:04
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
@ChristopherHogan @stevengj
Fix images in MPB data analysis tutorial (NanoComp#695)
e22ff80 
* Fix images in MPB data analysis tutorial

* Add matplotlib troubleshooting for mac

* Import meep before matplotlib to work around macOS issues

* Fix typo in docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

mpb_data_analysis.py produces incorrect images
3 participants
@ChristopherHogan @JohnWeiner @stevengj