Fix Python discovery issue when calling `sympy` solver from wheel

JCGoran commented 6 months ago

(Splitting this up from #1147 as it was getting too large).

As an example (stolen from one of the notebooks):

# example.py
import nmodl.dsl as nmodl

def run_conductance_visitor_and_return_breakpoint(mod_string):
    # parse NMDOL file (supplied as a string) into AST
    driver = nmodl.NmodlDriver()
    AST = driver.parse_string(mod_string)
    # run SymtabVisitor to generate Symbol Table
    nmodl.symtab.SymtabVisitor().visit_program(AST)
    # constant folding, inlining & local variable renaming passes
    nmodl.visitor.ConstantFolderVisitor().visit_program(AST)
    nmodl.visitor.InlineVisitor().visit_program(AST)
    nmodl.visitor.LocalVarRenameVisitor().visit_program(AST)
    # run CONDUCTANCE visitor
    nmodl.visitor.SympyConductanceVisitor().visit_program(AST)
    # return new BREAKPOINT block
    return nmodl.to_nmodl(
        nmodl.visitor.AstLookupVisitor().lookup(
            AST, nmodl.ast.AstNodeType.BREAKPOINT_BLOCK
        )[0]
    )

ex1 = """
NEURON {
    USEION na READ ena WRITE ina
    RANGE gna
}
BREAKPOINT {
    ina = gna*(v - ena)
}"""
print(run_conductance_visitor_and_return_breakpoint(ex1))

Running this with a freshly-installed wheel on latest master (built with python setup.py bdist_wheel) gives:

$ python example.py
[NMODL] [critical] :: NMODL_PYLIB environment variable must be set to load embedded python
Traceback (most recent call last):
  File "/private/var/folders/r_/1lcd8hvd6nzgtzcy8wwq9slx186djn/T/tmp.vzUItTIK/example.py", line 32, in <module>
    print(run_conductance_visitor_and_return_breakpoint(ex1))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/private/var/folders/r_/1lcd8hvd6nzgtzcy8wwq9slx186djn/T/tmp.vzUItTIK/example.py", line 15, in run_conductance_visitor_and_return_breakpoint
    nmodl.visitor.SympyConductanceVisitor().visit_program(AST)
RuntimeError: NMODL_PYLIB not set

Basically, due to the way loading of Python is handled, we should set the various env variables as soon as we load the module, otherwise the user may find mysterious errors.

Other changes:

updated requirements and setup.py (for importlib)
added a test for env variables (needs a BREAKPOINT statement to activate the sympy solver)
added automatic setting of NMODL_PYLIB and NMODLHOME env variables (only if they are not already set!)

bbpbuildbot commented 6 months ago

Logfiles from GitLab pipeline #195357 (:no_entry:) have been uploaded here!

Status and direct links:

JCGoran commented 6 months ago

Need https://github.com/BlueBrain/spack/pull/2320 to get merged, then the gitlab pipeline should be ✅ EDIT: Spack PR got merged, all green now :)

bbpbuildbot commented 6 months ago

Logfiles from GitLab pipeline #195420 (:no_entry:) have been uploaded here!

Status and direct links:

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.40%. Comparing base (bc913ef) to head (8e7c5ca).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1165 +/- ## ======================================= Coverage 88.40% 88.40% ======================================= Files 175 175 Lines 12934 12934 ======================================= Hits 11434 11434 Misses 1500 1500 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bbpbuildbot commented 6 months ago

Logfiles from GitLab pipeline #195522 (:white_check_mark:) have been uploaded here!

Status and direct links:

bbpbuildbot commented 6 months ago

Logfiles from GitLab pipeline #195710 (:white_check_mark:) have been uploaded here!

Status and direct links:

bbpbuildbot commented 6 months ago

Logfiles from GitLab pipeline #195916 (:white_check_mark:) have been uploaded here!

Status and direct links:

bbpbuildbot commented 6 months ago

Logfiles from GitLab pipeline #195934 (:white_check_mark:) have been uploaded here!

Status and direct links:

bbpbuildbot commented 6 months ago

Logfiles from GitLab pipeline #195941 (:white_check_mark:) have been uploaded here!

Status and direct links:

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.17%. Comparing base (2cff7e0) to head (f8d41dd).

:exclamation: Current head f8d41dd differs from pull request most recent head 62b93f0. Consider uploading reports for the commit 62b93f0 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1165 +/- ## ======================================= Coverage 87.17% 87.17% ======================================= Files 175 175 Lines 12884 12884 ======================================= Hits 11232 11232 Misses 1652 1652 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JCGoran commented 6 months ago

Minor annoyance: if we merge this, we need to add py-importlib-resources to the NMODL Spack recipe (only if we test on Python 3.8, if not, you can ignore this).

1uc commented 6 months ago

That doesn't seem wrong. The dependencies changed, hence the Spack recipe needs to change.

BlueBrain / nmodl

Fix Python discovery issue when calling `sympy` solver from wheel #1165

Codecov Report

Codecov Report