pyiron / pyiron_workflow

Graph-and-node based workflows
BSD 3-Clause "New" or "Revised" License
12 stars 1 forks source link

Registration #147

Closed liamhuber closed 10 months ago

liamhuber commented 10 months ago

Ahead of saving and loading workflows, we need the saved nodes to be able to know what node package they came from, so that workflows can register these on load, re-create the nodes, then the nodes can load their state from the data. (Note that Macros don't have the same problem, because they know how to create all their children and do so at initialization, so they can simply read off their saved states -- there is an edge case where a macro has replaced one or more of it's children, but we can deal with that later).

In principle each node package should have a unique identifier, but in practice I'm still just using the import module, e.g. "pyiron_workflow.node_library.standard". This "identifier" now gets stored on the node classes when they are registered as a package, so they'll be available to save later.

Otherwise there's a bunch of refactoring, including moving more responsibility onto NodePackage and making the (for now) equivalence of the package identifier and module name more clear.

There's also a bit of extra syntactic sugar so that you can register stuff to a deeply nested domain from the get-go, e.g. Workflow.create.register("my.deep.domain", "pyiron_workflow.node_library.standard"); Workflow.create.my.deep.domain.Add().

review-notebook-app[bot] commented 10 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

github-actions[bot] commented 10 months ago

Binder :point_left: Launch a binder notebook on branch _pyiron/pyiron_workflow/registrationtracking

codacy-production[bot] commented 10 months ago

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
:white_check_mark: +0.04% (target: -1.00%) :white_check_mark: 95.56%
Coverage variation details | | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (1ea4ff69e0eed921e625cbef519e93d72eec0363) | 2503 | 2136 | 85.34% | | | Head commit (d44217bb869dbc7f4744aad1362aa0f8c2a78d15) | 2559 (+56) | 2185 (+49) | 85.38% (**+0.04%**) | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `
Diff coverage details | | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#147) | 90 | 86 | **95.56%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings    Change summary preferences

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

github-actions[bot] commented 10 months ago

Pull Request Test Coverage Report for Build 7426980580


Totals Coverage Status
Change from base Build 7426966556: 0.04%
Covered Lines: 4646
Relevant Lines: 5145

💛 - Coveralls
liamhuber commented 10 months ago

Notebook failure appears to just be a versioning inconsistency with pyiron/atomistics; I'll fix it before merging but it doesn't impact the meat here

liamhuber commented 10 months ago

Also, forgive me @samwaseda, for this PR is sinfully big. I should have decomposed it into some smaller steps, but I rushed. I hope the readability of the end product is high enough to compensate somewhat.

liamhuber commented 10 months ago

Original notebook error seems to have been a caching error and outdated version of pyiron/atomistics. Now pyiron_atomistics is breaking in a strange (and still seemingly unrelated) way:

File /usr/share/miniconda3/envs/my-env/lib/python3.11/site-packages/pyiron_base/jobs/job/runfunction.py:700, in handle_failed_job(job, error)
    698     if job.server.run_mode.non_modal:
    699         state.database.close_connection()
--> 700     raise RuntimeError("Job aborted")
    701 else:
    702     return True, out

RuntimeError: Job aborted
liamhuber commented 10 months ago

One problem was that updates to the extras_require in setup.py are not being automatically propagated to .ci_support/notebooks-env.yml. This is a CI bug, and I opened an issue for it: #148

liamhuber commented 10 months ago
Looking for: ['jupyter', 'papermill', 'coveralls', 'coverage', 'bidict=0.22.1', 'cloudpickle=3.0.0', 'graphviz=8.1.0', 'matplotlib=3.8.2', 'pympipool=0.7.9', 'python-graphviz=0.20.1', 'toposort=1.10', 'typeguard=4.1.5', 'ase=3.22.1', 'atomistics=0.1.20', 'lammps', 'phonopy=2.21.0', 'pyiron_atomistics=0.4.1', 'pyiron-data=0.0.24', 'numpy=1.26.2']

Could not solve for environment specs
The following packages are incompatible
├─ atomistics 0.1.20**  is requested and can be installed;
└─ pyiron_atomistics 0.4.1**  is uninstallable because it requires
   └─ atomistics >=0.1.12,<=0.1.18 , which conflicts with any installable versions previously reported.
Error: Process completed with exit code 1.

Right, the dependabot upgrade of atomistics to 0.1.19 got through the tests before because it only impacts the notebooks test, but changes to setup.py by dependabot were not getting propagated to the actual notebooks env so the conflict never arose. I'm going to go patch this in a separate PR.

codacy-production[bot] commented 10 months ago

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
:white_check_mark: +0.04% (target: -1.00%) :white_check_mark: 95.56%
Coverage variation details | | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (908a72470e74e82ec7c86e0d9cab1cbf4d1ab0af) | 2503 | 2136 | 85.34% | | | Head commit (1964aa74ee6b50102040e7963fa6b495b05988d9) | 2559 (+56) | 2185 (+49) | 85.38% (**+0.04%**) | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `
Diff coverage details | | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#147) | 90 | 86 | **95.56%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings    Change summary preferences

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

liamhuber commented 10 months ago

Closes #152