Should workflow logic stay inside API code or move into `amorphouspy` package?

[ltalirz]

So far, amorphouspy has no dependency on a workflow manager (except one test that uses executorlib - perhaps we should drop it there).
This is great, since users can easily integrate the package into their workflow system of choice.

At the same time, this means the full pipeline "composition" -> "properties" (which uses executorlib) currently lives somewhat hidden inside the API in the folder below

amorphouspy/amorphouspy_api/src/amorphouspy_api/workflows/__init__.py

Lines 13 to 20 in b73ea7f

	STEPS: dict[str, AnalysisFn] = {
	"structure_generation": generate_structure,
	"melt_quench": run_melt_quench,
	"structure": run_structural_analysis,
	"viscosity": run_viscosity,
	"cte": run_cte,
	"elastic": run_elastic,
	}

So far, the pipeline does not do much besides chaining the individual steps in a sensible parallel way and preparing data for plots, but I suspect that over time more and more logic will build up there (e.g. around error recovery). I suspect this will be valuable not only for users of the API but also for users of amorphouspy who want to screen a bunch of compositions.

I would suggest we leave this question open until we've had some feedback on this question from users.
In the meanwhile, we should try whenever possible to add logic concerning choosing sensible defaults, and even error recovery at LAMMPS level inside amorphouspy rather than at the workflow level in the API.

[ltalirz]

This question popped up again in #293 where we would like to do parametric melt-quench and in #174 where we want to parallelize the calculation of the viscosity.

I don't think it makes sense to move everything that requires a workflow manager into the web API code - a parallelized viscosity workflow is just as useful for direct users of amorphouspy.

That leaves the question of how to handle such workflows that necessarily have a dependency on executorlib because they involve parallelization.

1. I think it makes sense to keep them a bit separate, but where to put them - e.g. in a subfolder of amorphouspy or into a separate amorphouspy_workflows/amorphouspy_parallel package

• I don't have strong feelings here

2. Should we aim to make these workflows work also if executorlib is not installed (by having some kind of default executor that just runs stuff in serial)?

• Since executorlib uses the interface of executors from the standard library, it might be possible - but executorlib adds functionality that is not supported by the standard lib executors (which would be ignored/lead to conflicts). So, I would rather require executorlib for use of these workflows

@jan-janssen @Atilaac @Gitdowski Any thoughts / further ideas?

[jan-janssen]

I still see this as a perfect application for the Python Workflow Definition. We could have a jupyter notebook which demonstrates how to setup the workflow with executorlib and run it, as a demonstration how people can use it in practice. The advantage of the jupyter notebook would be it could be used as documentation, integration test and interactive demonstration on a platform like mybinder. Then as part of this notebook we could also export the workflow to the Python Workflow definition JSON format. These exported workflows could then be used inside the API by just referencing the JSON file.

[ltalirz]

As discussed in today's meeting, we propose to

1. Clean up the structure of amorphouspy/
2. Add a new folder in amorphouspy/ (like parallel/ / ...)
3. Put code dependent on executorlib there and have an import guard in that subfolder that checks for existence of executorlib
4. Add a pixi command to automatically export the workflows defined there in python workflow definition format

We discussed the option to directly use the Python workflow definition JSON files in the jobs API, but decided against it due to factors like maintainability, code duplication, overhead during execution and lack of support to define computational resources for workflow steps.

[ltalirz]

Hey @Atilaac , I thought a bit about the package structure and came up with the proposal below.

amorphouspy/
├── lammps/                    # Infrastructure: how to talk to LAMMPS
│   ├── potentials/
│   ├── io/                    
│   ├── runner.py              # _run_lammps_md, error capture, command builder
│   └── md.py                  # generic single-point MD
|
├── fabrication/               # Science: how to make an amorphous structure
├── ├── neighbors.py
│   ├── composition.py
│   ├── density.py
│   ├── geometry.py
│   ├── planner.py
│   ├── meltquench.py
│   └── meltquench_protocols.py
│
├── properties/                # Science: how to measure properties
│   ├── structural/
│   │   ├── rdf.py
│   │   ├── bond_angles.py
│   │   ├── rings.py
│   │   ├── qn.py
│   │   ├── cavities.py
│   │   ├── structure_factor.py
│   │   └── all.py
│   ├── elastic.py
│   ├── cte.py
│   └── viscosity.py
│
├── pipelines/                 # Orchestration: how to run it at scale
│   └── ...                    # composes fabrication + characterization
│                              # via executorlib

I thought I would ask for your thoughts before implementing it

One weak point I see is that high-temparture viscosity is not really a property of a structure but of the composition; anyhow I think this inconsistency is acceptable

[Atilaac]

Thank @ltalirz for the proposal. Check the proposition below from my side, it is comparable to yours with only a few modifications.

amorphouspy/
│
├── atoms/
│   ├── __init__.py
│   ├── composition.py
│   ├── density.py
│   ├── mass.py
│   ├── neighbors.py
│   └── shared.py
│
├── potentials/
│   ├── __init__.py
│   ├── _config.py
│   ├── potential.py
│   ├── pmmcs_potential.py
│   ├── bmp_potential.py
│   ├── shik_potential.py
│   ├── bjp_potential.py
│   ├── du_teter_potential.py
│   └── yang_potential.py
│
├── lammps/
│   ├── __init__.py
│   ├── runner.py
│   └── io.py
│
├── fabrication/
│   ├── __init__.py
│   ├── geometry.py
│   ├── planner.py
│   ├── meltquench.py
│   └── protocols.py
│
├── simulation/
│   ├── __init__.py
│   ├── md.py
│   ├── elastic.py
│   ├── cte.py
│   └── viscosity.py
│
├── characterization/
│   ├── __init__.py
│   ├── rdf.py
│   ├── bond_angles.py
│   ├── rings.py
│   ├── qn.py
│   ├── cavities.py
│   ├── projected_rdf.py
│   ├── structure_factor.py
│   ├── averaging.py
│   ├── cte.py
│   └── all.py
│── pipelines/                 # Orchestration: how to run it at scale
│   └── ...                    # composes fabrication + characterization
│                              # via executorlib
└── __init__.py

[ltalirz]

Cheers @Atilaac !

Thanks for the suggestion with atoms/ - indeed I left out some smaller files from my proposal, and atoms/ can be a way to collect element/ASE-atoms type shared functionality.

I also considered the split between simulation and characterization before, but I would argue there is no clear line - they are all material properties, and in the progression "structural characterization", "elastic", "cte", "viscosity" they simply depend less and less on the starting structure.
Even the line MD/geo-opt vs. structural characterization "without simulation" is no longer a clear one, if we start to do some averaging over frames in the structural analysis (which I think we should).

I also find the term "characterization" on its own lacks specificity (it could just as well refer to a measurement like viscosity) and find properties.structural clearer. I would therefore propose to stick with the rather general fabrication vs properties split