Use the pympipool executor

[liamhuber]

Closes #75

Replaces Node.executor = True/False with providing actual executor instances (while keeping infrastructure support for instead instructing how to build an executor in the future, for nesting/reconnection/submitters).

The only big catch right now is that the tests hang on my local machine with the pympipool executor (while they run fine for the built-in and much simpler/weaker CloudpickleProcessPoolExecutor). This happens whenever a macro/workflow with children is the one getting an executor -- all the tests where function nodes have an executor work totally fine with both executors. Since this might be related to pympipool tests hanging on mac, I'm just going to naively push this PR and see if the tests pass OK on linux and windows. Additionally, the hanging tests work totally fine when run in a jupyter notebook -- so I'm optimistic this is something that can be overcome, I just need more data to figure out how.

TODO:

• Update docs
• Update demo notebook (at least the quickstart still uses executor = True
• Go look at pympipool for how the flux and slurm guys get tested and try to do something similar here I'm shoving this to "outside scope" for this PR, getting the PyMPI executor working is enough for now

[github-actions]

👈 Launch a binder notebook on branch pyiron/pyiron_workflow/use_pympipool

[liamhuber]

File "/usr/share/miniconda3/envs/my-env/lib/python3.10/site-packages/pympipool/backend/serial.py", line 30, in main
    input_dict = interface_receive(socket=socket)
  File "/usr/share/miniconda3/envs/my-env/lib/python3.10/site-packages/pympipool/shared/communication.py", line 147, in interface_receive
    return cloudpickle.loads(socket.recv())
ModuleNotFoundError: No module named 'test_macro'

This is the cause of failure for a couple of these. Might be those awkward expectations pympipool has on the PYTHONPATH that I mentioned somewhere else (the linked issue maybe?)

[liamhuber]

Ah, perhaps it runs in the notebook because it recognizes that things are being defined in __main__ and so knows to use by-value serialization, but when I'm running the tests they're in a .py file and so thinks that by-reference serialization will it doesn't realize that somehow this py file is not actually in the python path.

[liamhuber]

Ok, failures are from referencing objects defined in the same file but different scopes, such that the executor thinks it's something that should be available by reference but really it still needs to be pickled by value. Cf. this issue over on pympipool for a minimal(ish) example.

I can get around this by modifying the existing tests so they never have this same-file-different-scope problem, but I don't want some user/node dev to run into this problem later so let's see if it can be intelligently fixed upstream before giving up.

[liamhuber]

pympipool is adding "." to the python path; the claim is that this is for flux, but could it be needed for other executors? The tests there involve a cd tests; python -m unittest discover ., so "." is directly the tests folder. In contrast, the centralized CI is directly invoking coverage run -m unittest discover ${{ inputs.test-dir }}, where the directory being passed in is the default value for the pyiron repos using the reusable workflows, i.e. tests/unit. So in my case I imagine "." is actually the main repository directory, which is not a python module.

Is there an equivalent explanation for the coverage tests? Yes, it similarly runs the central reusable workflow with its default value of tests s.t. the final command is coverage run -m unittest discover tests and still not cd tests; python -m unittest discover ..

To test this hypothesis, let's make sure tests/unit is part of the path before we use the executor s.t. test_macro is visible for import.

[liamhuber]

That did nothing, I'm getting the exact same ModuleNotFoundErrors. I should look to make sure I'm actually getting the path expected.

[liamhuber]

The path is as expected. Maybe the path needs to be added in another process too? I am now trying just manually using a pympipool test (mostly), where we first cd and then discover tests in ..

[liamhuber]

Ok, it looks like the cd then discover . worked -- it ran into errors discovering the static package, but it finished cleanly and the only errors had nothing to do with the executor:

======================================================================
ERROR: test_creator_access_and_registration (test_composite.TestComposite.test_creator_access_and_registration)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/share/miniconda3/envs/test/lib/python3.11/site-packages/pyiron_workflow/interfaces.py", line 205, in _verify_identifier
    module = import_module(package_identifier)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/share/miniconda3/envs/test/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1126, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1140, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'static'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/pyiron_workflow/pyiron_workflow/tests/unit/test_composite.py", line 71, in test_creator_access_and_registration
    self.comp.register("demo", "static.demo_nodes")
  File "/usr/share/miniconda3/envs/test/lib/python3.11/site-packages/pyiron_workflow/composite.py", line 517, in register
    cls.create.register(domain=domain, package_identifier=package_identifier)
  File "/usr/share/miniconda3/envs/test/lib/python3.11/site-packages/pyiron_workflow/interfaces.py", line 163, in register
    self._verify_identifier(package_identifier)
  File "/usr/share/miniconda3/envs/test/lib/python3.11/site-packages/pyiron_workflow/interfaces.py", line 212, in _verify_identifier
    raise ValueError(
ValueError: The package identifier is static.demo_nodes is not valid. Please ensure it is an importable module with a list of Node objects stored in the variable `nodes`.

======================================================================
ERROR: test_registration (test_interfaces.TestCreator.test_registration)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/share/miniconda3/envs/test/lib/python3.11/site-packages/pyiron_workflow/interfaces.py", line 205, in _verify_identifier
    module = import_module(package_identifier)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/share/miniconda3/envs/test/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1126, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1140, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'static'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/pyiron_workflow/pyiron_workflow/tests/unit/test_interfaces.py", line 27, in test_registration
    self.creator.register("demo", "static.demo_nodes")
  File "/usr/share/miniconda3/envs/test/lib/python3.11/site-packages/pyiron_workflow/interfaces.py", line 163, in register
    self._verify_identifier(package_identifier)
  File "/usr/share/miniconda3/envs/test/lib/python3.11/site-packages/pyiron_workflow/interfaces.py", line 212, in _verify_identifier
    raise ValueError(
ValueError: The package identifier is static.demo_nodes is not valid. Please ensure it is an importable module with a list of Node objects stored in the variable `nodes`.

----------------------------------------------------------------------

So it looks like the problem is that pympipool requires to first CD and then discover tests. IMO that's a problem with pympipool and not with pyiron_workflow. I'll go over there and see if I can reproduce this behaviour more simply with some new workflows and get help there.

[liamhuber]

Dammit, I can't invoke the reusable workflow as a step. That means, and the docs explicitly point this case out, that I can't modify the GITHUB_ENV of the called workflow.

If this problem can't be fixed directly in pympipool that will mean adding a step like below to the reusable workflow itself.

    - name: "Add test dirs to pythonpath (pympipool compliance)"
      run: |
        PWD=$(pwd)
        echo "PYTHONPATH=$PWD/tests/benchmark:$PYTHONPATH" >> $GITHUB_ENV
        echo "PYTHONPATH=$PWD/tests/integration:$PYTHONPATH" >> $GITHUB_ENV
        echo "PYTHONPATH=$PWD/tests/unit:$PYTHONPATH" >> $GITHUB_ENV

[liamhuber]

Per pympipool #239, I'm trying to modify the centralized CI to allow the python path to be expanded. Targeting the branch seems to have failed and push-pull didn't even run:

error parsing called workflow
".github/workflows/push-pull.yml"
-> "pyiron/actions/.github/workflows/push-pull-main.yml@tests_in_python_path"
: failed to fetch workflow: reference to workflow should be either a valid branch, tag, or commit

Ahaaaa, because I have a typo in "pythonpath"

[liamhuber]

Perfect! Worked for all three OSs. I updated the upstream branch to also apply the same trick for the coverage job, which is still hanging.

[liamhuber]

Note that once everything is settled, the workflow tags will need to be reset to main or a release.

[coveralls]

Pull Request Test Coverage Report for Build 7052576125

• 35 of 43 (81.4%) changed or added relevant lines in 3 files are covered.
• 69 unchanged lines in 8 files lost coverage.
• Overall coverage decreased (-0.8%) to 87.853%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pyiron_workflow/interfaces.py	16	24	66.67%

Files with Coverage Reduction	New Missed Lines	%
function.py	1	92.35%
io.py	2	90.23%
pyiron_workflow/io.py	2	85.71%
pyiron_workflow/util.py	2	84.62%
util.py	2	92.11%
composite.py	9	87.13%
interfaces.py	20	80.2%
node.py	31	87.5%

Totals
Change from base Build 7040333669:	-0.8%
Covered Lines:	3515
Relevant Lines:	4001

---

💛 - Coveralls

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.91% (target: -1.00%)	74.51%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (fa4d15b)	2006	1656	82.55%
Head commit (a8201e1)	2048 (+42)	1672 (+16)	81.64% (-0.91%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#77)	51	38	74.51%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences