materialsproject / pymatgen

Python Materials Genomics (pymatgen) is a robust materials analysis code that defines classes for structures and molecules with support for many electronic structure codes. It powers the Materials Project.
https://pymatgen.org
Other
1.51k stars 863 forks source link

bug in latest pymatgen #633

Closed computron closed 7 years ago

computron commented 7 years ago

System

Summary

Example code

from pymatgen.analysis.elasticity.elastic import *

Error message

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/ajain/Documents/code_matgen/pymatgen_repo/pymatgen/analysis/elasticity/__init__.py", line 7, in <module>
    from .elastic import *
  File "/Users/ajain/Documents/code_matgen/pymatgen_repo/pymatgen/analysis/elasticity/elastic.py", line 747, in <module>
    v_diff = np.vectorize(sp.diff)
NameError: name 'sp' is not defined

Suggested solution (if any)

computron commented 7 years ago

(note - if you don't like the import *, just do from pymatgen.analysis.elasticity.elastic import ElasticTensor and you get the same error)

montoyjh commented 7 years ago

Ah, I failed to protect a sympy-dependent section of the new code with the conditional. I'll fix shortly.

On Fri, Mar 31, 2017 at 11:36 AM, Anubhav Jain notifications@github.com wrote:

System

  • Pymatgen version: master
  • Python version: 2.7
  • OS version: mac

Summary

  • See example code

Example code

from pymatgen.analysis.elasticity.elastic import *

Error message

Traceback (most recent call last): File "", line 1, in File "/Users/ajain/Documents/code_matgen/pymatgen_repo/pymatgen/analysis/elasticity/init.py", line 7, in from .elastic import * File "/Users/ajain/Documents/code_matgen/pymatgen_repo/pymatgen/analysis/elasticity/elastic.py", line 747, in v_diff = np.vectorize(sp.diff) NameError: name 'sp' is not defined

Suggested solution (if any)

  • not sure what the intention is of the error-creating code, so no suggestions

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/materialsproject/pymatgen/issues/633, or mute the thread https://github.com/notifications/unsubscribe-auth/AGMZNHCSIeh_opp1aoGHVuxdjhfVA4-2ks5rrUedgaJpZM4MwAqk .

shyuep commented 7 years ago

Why is there a dependence on sympy?

shyuep commented 7 years ago

If sympy is really needed, we can just add it to dependency. It is not that large.

montoyjh commented 7 years ago

Just FYI, sympy is being used to determine the pseudoinverse expression in the higher order fitting scheme, which requires a symbolic derivative (this enables a lot more flexibility in handling different stress/strain inputs for fitting). I'm working on a refactor that doesn't require sympy, since it's by far the slowest piece of the procudure, but it's convenient to have the symbolic option.

On Mar 31, 2017 12:12 PM, "Shyue Ping Ong" notifications@github.com wrote:

Closed #633 https://github.com/materialsproject/pymatgen/issues/633 via b482fe2 https://github.com/materialsproject/pymatgen/commit/b482fe2ef81b72dab9d0bb570f58de4808eb938e .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/materialsproject/pymatgen/issues/633#event-1024623415, or mute the thread https://github.com/notifications/unsubscribe-auth/AGMZNDx71DclwoovlQnelKZF7rMmjX67ks5rrVAfgaJpZM4MwAqk .