pyiron / pyiron_ontology

Combining pyiron and owlready2 for ontologically guided workflow design
BSD 3-Clause "New" or "Revised" License
4 stars 0 forks source link

Rework #8

Closed liamhuber closed 1 year ago

liamhuber commented 1 year ago

Use a class-based paradigm for the mutual exclusion; lean heavily on individuals when defining new inputs/functions/nodes;

github-actions[bot] commented 1 year ago

Binder :point_left: Launch a binder notebook on branch _pyiron/pyironontology/rework

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

liamhuber commented 1 year ago

Black is failing for obvious reasons. In fact, because of the (IMO kind of ugly) owlready2 paradigm of just declaring a million pass classes, I'm not even sure I want to adhere to black here -- it would take up so much space.

pip check continues to fail because of the upstream conflict between pyiron_base and mendeleev, so nothing to do here.

The notebook failure is weird. I'm going to have to dig into it later.

$GITHUB_ACTION_PATH/../.support/build_notebooks.sh notebooks .ci_support/exclude
  shell: /usr/bin/bash -l {0}
  env:
    INPUT_RUN_POST: true
    CONDA: /usr/share/miniconda3
    CONDA_PKGS_DIR: /home/runner/conda_pkgs_dir
    DATE: 20230223
Error: Option '-k' requires an argument.
Error: Option '-k' requires an argument.
Error: Process completed with exit code 1.

It appears to refer to this line of the build_notebooks.sh script over in pyiron actions: papermill ${notebook} ${notebook%.*}-out.${notebook##*.} -k $kernel || i=$((i+1));, but I'm not sure why since the same line causes no trouble over in ironflow. It's true though that the script itself does not define a $kernel variable.

liamhuber commented 1 year ago

The notebook failure is weird. I'm going to have to dig into it later.

The kernel variable for papermill used to be set right in the script (depending whether we wanted to run with python 2.7 or 3). Now we only support >=3.8, so I just got rid of it altogether over in the actions repo.

liamhuber commented 1 year ago

Ok, the example notebook is still crashing when I try to run Murnaghan. I manually put in a print and it's using the same potential as I get locally, RuntimeError: Got an error running with 1986--Foiles-S-M--Ag-Au-Cu-Ni-Pd-Pt--LAMMPS--ipr1, so it's not crashing because it's using a potential with an invalid format or anything.

I did find that on MyBinder I get stuck with pyiron_atomistics 0.2.34 and pyiron_base 0.4.5. These are substantially behind the latest conda releases, I guess because of the mendeelev/base incompatibility on the sql library. Let's try pinning the atomistics depenency

liamhuber commented 1 year ago

Aha. '/usr/share/miniconda3/envs/my-env/share/pyiron/lammps/bin/run_lammps_2020.03.03.sh: line 2: exec: lmp_serial: not found\n'.

It turns out that I forgot to direct the CI to the local notebooks env.

I'm ok with the default behaviour (the centralized CI notebooks env and the local env) but it is a bit of a gotcha that should probably be highlighted in the actions documentation -- you can specify custom additional env files, you you do need to actually specify them!

liamhuber commented 1 year ago

Ok, super. The pip issue is, as mentioned, upstream.

I want to think a bit on whether or not to abandon black formatting though. Once I've made up my mind I'll either merge or black&merge.