First prototype of a code base #14

agoscinski · 2024-07-22T07:36:31Z

This is PR #12 with some additional changes that is based from the init-repo, so we can separate the repo structure and code base initialization. The module structure is

src
└── wcflow
    ├── core.py
    ├── __init__.py
    ├── _utils.py
    ├── wc
    │   ├── _classes.py
    │   ├── __init__.py
    │   └── _schema.py
    └── workgraph.py

The rough idea is that everything in the wc submodule is related to parsing of the file and converting it to a core.Workflow. The core.Workflow has all information to allow a convenient unrolling of the tasks over the periods. The workgraph.* is responsible for creating an AiiDA workgraph from a core.Workflow.

TODOs/Issues:

At the moment we run verdi presto before the tests to set up a profile, would be nicer if we could set up a profile within the tests
sphinx -> mkdocs

agoscinski · 2024-08-28T09:45:21Z

Notes from the meeting with @leclairm

We go with the schema library and pyaml, ditching strictyaml, because the anchors are still useful for our case.

We might want to change naming to

wc.Data -> wc.DataSpec
core.Data -> core.DataSpec
core.UnrolledData -> core.Data

@leclairm will rethink this

Some thoughts about relaxing the logic that a lagged date is only enforced if it is after the start_date

inputs:
  - icon_input:
    lag:
      condition: "possible" # other options could be "always" so the file should always exist
      value: '-P4M'

But for now it is okay

GeigerJ2

Alright, went once through the whole thing. Again, great work, @agoscinski! 🚀 A lot to digest, so I might have missed things. My comments consist mainly of questions for myself that would be nice to discuss for clarification.

As mentioned on Slack, when running the example, it's stuck. This might be the bug I found in the AiidaWorkGraph, where for output in task.unrolled_inputs:. Will test tomorrow. Apart from that, once we settle the strictyaml discussion, I'd say LGTM, and then we can further iterate.

examples/README.rst

GeigerJ2 · 2024-09-04T18:39:33Z

examples/files/configs/example_config.yaml

+      icon:
+        inputs:
+          - grid_file:
+              arg_option: "-g"


Won't be needed with actual ICON, I suppose.

How we integrate plugins is still an open question. But with the aiida builder we can get call input arguments and could technically support them, at least we can specify all orm data types that can be specified in a yaml file which should be most of them.

GeigerJ2 · 2024-09-04T18:46:34Z

src/wcflow/_utils.py

+class TimeUtils:
+    @staticmethod
+    def duration_is_less_equal_zero(duration: Duration) -> bool:
+        if (


Why not just <=?

It is not so easy to see on first glance but if the condition is: Even all are zero¿ or any is less than zero. If you do <= with the connection with and conditions, then all would need to be less than zero, so (-1, 1, 1, 1, 1, 1) would note be fulfilled. If you use or, then once one is zero the condition is already fulfilled, so (0, 1, 1, 1, 1, 1) would be fulfilled.

GeigerJ2 · 2024-09-04T18:47:20Z

src/wcflow/cmdline.py

@@ -0,0 +1,21 @@
+import argparse
+
+from aiida.manage.configuration import load_profile


Can just do from aiida import load_profile?

My pyright was unhappy with that^^. I general prefer for source code to be more verbose with their modules, but no strong opinions.

GeigerJ2 · 2024-09-04T18:47:53Z

src/wcflow/cmdline.py

+
+    aiida_workgraph = workgraph.AiidaWorkGraph(core_workflow)
+    print("Starting workflow please run `workgraph web start` in another terminal to start a GUI as webservice.")  # noqa: T201
+    result = aiida_workgraph.run()


Does it work with submit()?

Should work. It is just that then one needs to have rabbitmq for running the tests. Therefore I thought to switch to pixi, since there I can install all this in the background so nonaiida user do not need to bother.

GeigerJ2 · 2024-09-04T19:10:24Z

src/wcflow/wc/_classes.py

+    def __init__(
+        self,
+        name: str,
+        inputs: list[str | dict] | None = None,


Task class doesn't have inputs and outputs? Is Task like a raw representation, directly from the YAML file? I suppose CycleTask has a different purpose, as it also doesn't inherit from Task? Maybe you can explain in the meeting.

Is Task like a raw representation, directly from the YAML file?

Yes the class CycleTask represents a task in a cycle, while the python class Task represents a task definition outside of the cycles.

Should add this to the doc.

GeigerJ2 · 2024-09-04T19:12:55Z

src/wcflow/wc/_classes.py

+
+class Cycle(_NamedBase):
+    """
+    We never need to create instances of a cycle class so we only contains static methods


Maybe you can explain in the meeting.

src/wcflow/wc/_classes.py

src/wcflow/workgraph.py

GeigerJ2 · 2024-09-04T19:17:31Z

tests/files/scripts/icon.py

@@ -0,0 +1,39 @@
+#!/usr/bin/env python


Should this be also under tests? Same script as for the example, right? It's here to then also be used for testing?

Yes it is used in the tests (see the test_config_small.yaml). I think need remove icon.sh and icon as they are not used anymore. Need to verify that they are not needed.

src/wcflow/wc/_classes.py

Co-authored-by: Julian Geiger <[email protected]>

agoscinski · 2024-09-18T08:37:49Z

There was a small bug in the UnrolledDependency (used from variable) that has been fixed. The example should work now.

Co-authored-by: Julian Geiger <[email protected]>

agoscinski force-pushed the init-repo branch from a815ca5 to 1ba5176 Compare July 23, 2024 18:21

agoscinski force-pushed the init-repo-code branch from a2ca8c0 to e030336 Compare July 23, 2024 18:24

agoscinski force-pushed the init-repo branch 2 times, most recently from b35576c to 2805125 Compare July 24, 2024 08:37

agoscinski force-pushed the init-repo-code branch 2 times, most recently from a505b48 to c392d73 Compare July 24, 2024 10:02

Initializes code for running a WC workflow with AiiDA

b1c0d72

agoscinski force-pushed the init-repo-code branch from c392d73 to b1c0d72 Compare July 24, 2024 10:33

GeigerJ2 requested changes Sep 4, 2024

View reviewed changes

agoscinski commented Sep 12, 2024

View reviewed changes

src/wcflow/wc/_classes.py Outdated Show resolved Hide resolved

agoscinski and others added 7 commits September 12, 2024 11:19

Apply suggestions from code review

ce5144b

Co-authored-by: Julian Geiger <[email protected]>

Update README.rst with AiiDA installation and profile info

7a26582

fix typo

633ed77

small update example README

f42101c

add submit to aiida_workgraph

f0c5525

fix recursive loop in __repr__ function

d2ecee9

fix wrong date for unrolled dependencies

fe64bf7

Update examples/README.rst

1943c5c

Co-authored-by: Julian Geiger <[email protected]>

agoscinski changed the title ~~Initialize code base~~ First prototype of a code base Nov 27, 2024

agoscinski closed this Nov 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First prototype of a code base #14

First prototype of a code base #14

agoscinski commented Jul 22, 2024 •

edited

Loading

agoscinski commented Aug 28, 2024

GeigerJ2 left a comment

GeigerJ2 Sep 4, 2024

agoscinski Sep 18, 2024

GeigerJ2 Sep 4, 2024

agoscinski Sep 18, 2024

GeigerJ2 Sep 4, 2024

agoscinski Sep 18, 2024

GeigerJ2 Sep 4, 2024

agoscinski Sep 18, 2024

GeigerJ2 Sep 4, 2024

agoscinski Sep 18, 2024

GeigerJ2 Sep 4, 2024

GeigerJ2 Sep 4, 2024

agoscinski Sep 18, 2024

agoscinski commented Sep 18, 2024

		@@ -0,0 +1,21 @@
		import argparse

		from aiida.manage.configuration import load_profile

First prototype of a code base #14

First prototype of a code base #14

Conversation

agoscinski commented Jul 22, 2024 • edited Loading

agoscinski commented Aug 28, 2024

GeigerJ2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agoscinski commented Sep 18, 2024

agoscinski commented Jul 22, 2024 •

edited

Loading