Consume Protobuf events from dispatcher and pass them to the statemachine #3733

xjules · 2022-08-10T14:48:52Z

Issue
Resolves #3699

Approach
This contains the following sketches:

state machine - first draft of it, currently replacing statuses and propagating STEP_SUCCESS upwards. NOTE: this PR doesn't solve state machine issue (Design and implement experiment statemachine #3407) but requires a minimum running version of the state machine to process the protobuf events.
currently only reporter/dispatcher and JobQueue produce protobuf events. The other sources will follow after this PR is merged: Use Experiment server in all run models when feature is enabled #3768

mypy and pylint requires additional dependencies for type-checking, wherein mypy stabs needs to be compiled first though (check typing.yml workflow).

Pre review checklist

Added appropriate release note label
PR title captures the intent of the changes, and is fitting for release notes.
Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

codecov-commenter · 2022-08-11T13:04:26Z

Codecov Report

Merging #3733 (ca2ae4d) into main (283d7f1) will increase coverage by 0.14%.
The diff coverage is 90.47%.

❗ Current head ca2ae4d differs from pull request most recent head d7c9c2e. Consider uploading reports for the commit d7c9c2e to get more accurate results

@@            Coverage Diff             @@
##             main    #3733      +/-   ##
==========================================
+ Coverage   58.74%   58.88%   +0.14%     
==========================================
  Files         539      541       +2     
  Lines       40101    40255     +154     
  Branches     3638     3638              
==========================================
+ Hits        23557    23706     +149     
- Misses      15531    15536       +5     
  Partials     1013     1013

Impacted Files	Coverage Δ
...rc/ert/shared/models/iterated_ensemble_smoother.py	`59.37% <20.00%> (-0.76%)`	⬇️
src/ert/shared/models/ensemble_experiment.py	`92.75% <62.50%> (-2.64%)`	⬇️
src/ert/experiment_server/_server.py	`76.69% <63.15%> (-3.72%)`	⬇️
src/ert/shared/models/ensemble_smoother.py	`93.02% <75.00%> (-1.29%)`	⬇️
src/ert/shared/models/base_run_model.py	`88.50% <92.85%> (-0.34%)`	⬇️
src/ert/ensemble_evaluator/builder/_legacy.py	`92.62% <96.15%> (+0.24%)`	⬆️
src/ert/com_protocol/_state_machine.py	`98.13% <98.13%> (ø)`
src/ert/_c_wrappers/job_queue/queue.py	`86.52% <100.00%> (+0.17%)`	⬆️
src/ert/com_protocol/__init__.py	`100.00% <100.00%> (ø)`
src/ert/com_protocol/status_type_enum.py	`100.00% <100.00%> (ø)`
... and 7 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

.github/workflows/typing.yml

src/ert/experiment_server/_server.py

src/ert/experiment_server/_state_machine.py

dev-requirements.txt

.github/workflows/typing.yml

src/ert/_c_wrappers/job_queue/queue.py

berland · 2022-10-13T06:51:35Z

src/ert/ensemble_evaluator/builder/_legacy.py

+    ]:
+        if experiment_id is not None:
+
+            def func(


maybe call this something more descriptive, like node_status ? (since we use a node_status_builder to make it)

I renamed both: node_builder and event_builder respectivelly.

berland · 2022-10-13T06:51:54Z

src/ert/ensemble_evaluator/builder/_legacy.py

+
+        else:
+
+            def func2(status: str, real_id: Optional[int] = None) -> CloudEvent:


def cloudevent ?

src/ert/ensemble_evaluator/builder/_legacy.py

src/ert/experiment_server/_server.py

berland · 2022-10-17T08:36:12Z

Noting a crash:

$ ert es_mda --enable-experiment-server poly.ert
Experiment failed: Phase must be an integer between (inclusive) 0 and 2, got 3.
ERT crashed unexpectedly with "Phase must be an integer between (inclusive) 0 and 2, got 3.".
See logfile(s) for details:
   /home/berland/projects/ert/test-data/poly_example/logs/ert-log-2022-10-17T1033.txt

(this includes a bug-fix on line 240 of base_run_model where an f-string misses an 'f')

src/ert/shared/models/base_run_model.py

src/ert/shared/models/ensemble_experiment.py

berland · 2022-10-17T11:25:35Z

Noting a crash:

$ ert es_mda --enable-experiment-server poly.ert
Experiment failed: Phase must be an integer between (inclusive) 0 and 2, got 3.
ERT crashed unexpectedly with "Phase must be an integer between (inclusive) 0 and 2, got 3.".
See logfile(s) for details:
   /home/berland/projects/ert/test-data/poly_example/logs/ert-log-2022-10-17T1033.txt

(this includes a bug-fix on line 240 of base_run_model where an f-string misses an 'f')

This crash exists prior to this PR.

src/ert/com_protocol/_state_machine.py

types-requirements.txt

src/ert/com_protocol/_state_machine.py

berland · 2022-10-18T07:38:59Z

The file tests/ert_tests/cli/test_integration_cli.py has some experiment server tests at the end which assert on stdout. Should these be rewritten to assert on something smarter? (or it might not make sense in this PR since the state machine is presumable gone when we get to the assert in those integration tests)

xjules · 2022-10-20T10:45:30Z

Agree, but as you said .. not in this PR :)

Resolves equinor#3699. Split base_run_model dispatch into CloutEvent and Protobuf versions. This requires to add mypy_protobuf, types-protobuf, pylint-protbuf as dependecies. Additionally when type checking we need to compile the protobuf stub file before type checking. Experiment also gets a new attribute _iter_map thats contains mapping between iter_num and ensemble_id. This also adds a basic implementation of state machine, which currently only overwrites status and propagates STEP_SUCCESS upwards in order to get the propert successful realizations number. The protobuf schema gets updates by dedicated statuses. A new helper function is introduced node_status_builder, which alleviates creation of distinct DispatcherMessages. Note that when experiment_id is set we switch sending CloudEvent to DispatcherMessages. All the protobuf handling along with the basic implementation of state machine is now located in its own _ert_com_protocol module. All based_run_models are able to run with `--enable-experiment-server` enabled.

xjules · 2022-10-21T09:55:27Z

Noting a crash:

$ ert es_mda --enable-experiment-server poly.ert
Experiment failed: Phase must be an integer between (inclusive) 0 and 2, got 3.
ERT crashed unexpectedly with "Phase must be an integer between (inclusive) 0 and 2, got 3.".
See logfile(s) for details:
   /home/berland/projects/ert/test-data/poly_example/logs/ert-log-2022-10-17T1033.txt

(this includes a bug-fix on line 240 of base_run_model where an f-string misses an 'f')

This crash exists prior to this PR.

It's fixed now

berland

Looking good, huge job!

xjules self-assigned this Aug 10, 2022

xjules force-pushed the state_machine_job branch 2 times, most recently from 73bd2c7 to df400a8 Compare August 18, 2022 13:33

xjules changed the title ~~Consume Protobuf events and pass them to the statemachine~~ Consume Protobuf events from dispatcher and pass them to the statemachine Aug 18, 2022

xjules force-pushed the state_machine_job branch 2 times, most recently from c7f2973 to e20e826 Compare August 24, 2022 13:29

xjules force-pushed the state_machine_job branch 5 times, most recently from 54b4519 to 74233ed Compare August 27, 2022 19:26

xjules mentioned this pull request Aug 30, 2022

POC for using Protobuf and gRPC #3568

Closed

xjules force-pushed the state_machine_job branch 3 times, most recently from 208aa17 to 057a3ce Compare August 31, 2022 15:30

berland reviewed Sep 1, 2022

View reviewed changes

.github/workflows/typing.yml Show resolved Hide resolved

berland reviewed Sep 1, 2022

View reviewed changes

src/ert/experiment_server/_server.py Outdated Show resolved Hide resolved

berland reviewed Sep 1, 2022

View reviewed changes

src/ert/experiment_server/_server.py Outdated Show resolved Hide resolved

berland reviewed Sep 1, 2022

View reviewed changes

src/ert/experiment_server/_state_machine.py Outdated Show resolved Hide resolved

berland reviewed Sep 1, 2022

View reviewed changes

src/ert/experiment_server/_state_machine.py Outdated Show resolved Hide resolved

xjules force-pushed the state_machine_job branch from 4af480d to 6ea0c71 Compare September 5, 2022 08:15

xjules added experiment-server release-notes:new-feature Automatically categorise as new feature in release notes labels Sep 5, 2022

xjules marked this pull request as ready for review September 5, 2022 08:27

xjules force-pushed the state_machine_job branch from 91a376e to 9154369 Compare September 6, 2022 09:38

berland reviewed Sep 6, 2022

View reviewed changes

dev-requirements.txt Show resolved Hide resolved

xjules force-pushed the state_machine_job branch from fcba0a6 to 87725d4 Compare September 6, 2022 18:03

berland reviewed Sep 7, 2022

View reviewed changes

.github/workflows/typing.yml Outdated Show resolved Hide resolved

berland reviewed Sep 7, 2022

View reviewed changes

src/ert/_c_wrappers/job_queue/queue.py Outdated Show resolved Hide resolved

berland reviewed Oct 13, 2022

View reviewed changes

src/ert/ensemble_evaluator/builder/_legacy.py Outdated Show resolved Hide resolved

berland reviewed Oct 13, 2022

View reviewed changes

src/ert/experiment_server/_server.py Show resolved Hide resolved

berland reviewed Oct 13, 2022

View reviewed changes

src/ert/experiment_server/_server.py Outdated Show resolved Hide resolved