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

SciPy is not imported, leading to errors in printing results #1

Closed
bocklund opened this issue Jul 6, 2018 · 2 comments
Closed

SciPy is not imported, leading to errors in printing results #1

bocklund opened this issue Jul 6, 2018 · 2 comments

Comments

@bocklund
Copy link

bocklund commented Jul 6, 2018

effmass/analysis.py imports integrate by name, however the mass_integration function uses the qualified scipy.integrate.quad. This leads to an error in line outputs.print_results(segments[-1], data, settings), which fails with the following traceback:

---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-26-204cc925e032> in <module>()
----> 1 outputs.print_results(segments[-1], data, settings)

~/Projects/joss-reviews/effmass/effmass/outputs.py in print_results(segment, data, settings, polyfit_order)
    136                   polyfit_order=polyfit_order)]))
    137     print("optical mass at band edge (assuming the Kane dispersion) is {:.2f}".
--> 138           format(segment.optical_effmass_kane_dispersion()))
    139 
    140     plt.figure(figsize=(8, 8))

~/Projects/joss-reviews/effmass/effmass/analysis.py in optical_effmass_kane_dispersion(self, fermi_level, temp, alpha, mass_bandedge, upper_limit)
    488             alpha=alpha,
    489             mass_bandedge=mass_bandedge,
--> 490             upper_limit=upper_limit)
    491         bottom = self.weight_integration(
    492             fermi_level=fermi_level,

~/Projects/joss-reviews/effmass/effmass/analysis.py in mass_integration(self, fermi_level, temp, alpha, mass_bandedge, upper_limit)
    334         fermi_level = self.fermi_energy if fermi_level is None else fermi_level
    335 
--> 336         result = scipy.integrate.quad(
    337             self._mass_integrand,
    338             0,

NameError: name 'scipy' is not defined

Fixing scipy.integrate.quad -> integrate.quad should fix this.

I also suggest writing a test for this code path.

(Part of openjournals/joss-reviews#797)

@bocklund
Copy link
Author

bocklund commented Jul 6, 2018

Also affects the lines where optical mass is calculated in the Jupyter Notebook examples, e.g. optical_mass_111 = [segments[-6].optical_effmass_kane_dispersion(fermi_level=data.CBM+burstein_moss_kane(x,average_mass,average_alpha),alpha=0.16/ev_to_hartree,mass_bandedge=0.155,upper_limit=0.1) for x in concentrations]

@lucydot
Copy link
Owner

lucydot commented Jul 9, 2018

Thank you for finding this bug @bocklund. I have fixed this as you suggested and have written a test to catch similar failures in the future.

@lucydot lucydot closed this as completed Jul 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants