New validation mode: via sending model #205

Parzival-05 · 2024-12-27T04:48:58Z

No description provided.

AIAgent/common/file_system_utils.py

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

…ault searcher

gsvgit · 2025-03-13T05:56:06Z

AIAgent/ml/dataset.py

@@ -492,6 +492,10 @@ def graphs_are_equal(new_g_v, old_g_v, new_s_v, old_s_v):
            return np.array_equal(new_g_v, old_g_v) and np.array_equal(new_s_v, old_s_v)

        def merge_distributions(old_distribution, new_distribution):
+            if old_distribution.device != new_distribution.device:


What is it?

The data for manipulation must be on the same device

gsvgit · 2025-03-13T05:59:31Z

AIAgent/ml/validation/coverage/game_managers/model/classes.py

+@dataclass(slots=True)
+class ModelGameStep:
+    GameState: GameState
+    Output: list


List of what?

gsvgit · 2025-03-13T06:05:09Z

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

+        game_map_info = ModelGameMapInfo(
+            total_game_state=None,
+            total_steps=[],
+            proc=None,  # type: ignore


Too much problems with types -> design is wrong.

gsvgit · 2025-03-13T06:06:27Z

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

+    ) -> Tuple[int, socket.socket]:
+        if attempts <= 0:
+            raise RuntimeError("Failed to occupy port")
+        logging.debug(f"Looking for port... Attempls left: {attempts}.")


Why this logic is not in connection manager (utils?)?

gsvgit · 2025-03-13T06:14:45Z

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

+                game_map_info.proc.wait()
+                with open(steps, "br") as f:
+                    steps_json = json.load(f)
+                steps = list(map(lambda v: ModelGameStep.from_dict(v), steps_json))  # type: ignore


Such tricks with types hint at bad design.

Pyright issue with dataclass_json methods lidatong/dataclasses-json#309

Try this: https://github.com/lidatong/dataclasses-json?tab=readme-ov-file#use-my-dataclass-with-json-arrays-or-objects

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

gsvgit · 2025-03-13T06:18:31Z

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

+                step = steps.pop(0)
+                game_state, nn_output = step.GameState, step.Output
+
+                def get_hetero_data(game_state: GameState, nn_output: list):


May be moved to dataset?

gsvgit · 2025-03-13T06:21:43Z

AIAgent/run_training.py

@@ -256,6 +257,13 @@ def objective(
            torch.save(model.state_dict(), CURRENT_MODEL_PATH)
            mlflow.log_artifact(CURRENT_MODEL_PATH, str(epoch))

+            model_kwargs = trial.params.copy()


What is it?

Model args are required by the onnx converter

PySymGym/AIAgent/onyx.py

Line 154 in becd6e7

help="Path to .yaml-file with model kwargs",

emnigma

A lot of TODOs still. Will they be fixed? Or you want to merge them?

emnigma · 2025-03-17T12:50:14Z

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

+    SVMConnectionInfo,
+)
+from ml.validation.coverage.game_managers.utils import set_timeout_if_needed
+from onyx import entrypoint, load_gamestate, resolve_import_model


Suggested change

from onyx import entrypoint, load_gamestate, resolve_import_model

from onyx import entrypoint as save_torch_model_to_onnx_file, load_gamestate, resolve_import_model

emnigma · 2025-03-17T12:51:50Z

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

+        self._create_onnx_model()
+        self._clean_output_folder()


swap please, if SVMS_OUTPUT_PATH == CURRENT_ONNX_MODEL_PATH.parent, then onnx model will be removed

emnigma · 2025-03-17T13:03:05Z

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

+                game_map_info.proc.wait()
+                with open(steps, "br") as f:
+                    steps_json = json.load(f)
+                steps = list(map(lambda v: ModelGameStep.from_dict(v), steps_json))  # type: ignore


Try this: https://github.com/lidatong/dataclasses-json?tab=readme-ov-file#use-my-dataclass-with-json-arrays-or-objects

emnigma · 2025-03-17T13:13:57Z

AIAgent/ml/validation/coverage/game_managers/model/process_game_manager.py

+            steps = None
+        return steps
+
+    def are_steps_required(self, game_map: GameMap, required: bool):


what is "pollute the class namespace"? If the issue is in duck typing hints, you can create static class and put these functions inside

I am suggesting this change because rn this function is unreadable to the reviewer. Also imagine you need to test convert_steps_to_hetero function. How would you do it?

emnigma · 2025-03-17T13:21:14Z

AIAgent/ml/validation/coverage/validate_coverage.py

@@ -57,12 +64,15 @@ def _evaluate_game_map(
            )
            map_name = game_map.MapName
            if self.dataset.is_update_map_required(map_name, map_result):
+                self._game_manager.are_steps_required(game_map=game_map, required=True)


why something is run after alert_svm_about_step_saving? According to this func documentation, "symbolic execution environment" is "notified", why something else is happening?

Parzival-05 force-pushed the david/model-validation branch 5 times, most recently from 04b5e54 to 1dfc4de Compare December 27, 2024 22:10

Parzival-05 requested review from emnigma, gsvgit, Anya497, ancavar and ksenmel December 27, 2024 22:42

Parzival-05 force-pushed the david/model-validation branch 2 times, most recently from 7913201 to 9dc7cf6 Compare December 28, 2024 18:45

Parzival-05 force-pushed the david/new-validation-design branch from ae4f954 to 3080a6d Compare December 28, 2024 19:46

Parzival-05 force-pushed the david/model-validation branch from 9dc7cf6 to 0da98e9 Compare December 28, 2024 19:50

Parzival-05 force-pushed the david/new-validation-design branch from 3080a6d to 8d465dc Compare December 28, 2024 20:34

Parzival-05 force-pushed the david/model-validation branch from 0da98e9 to 2d61059 Compare December 28, 2024 21:00

Parzival-05 marked this pull request as draft December 28, 2024 22:07

Parzival-05 force-pushed the david/model-validation branch 2 times, most recently from 1087af2 to 2bdb53b Compare December 30, 2024 04:59

Parzival-05 force-pushed the david/new-validation-design branch from becaff2 to 843076a Compare December 30, 2024 05:01

Parzival-05 marked this pull request as ready for review December 30, 2024 05:34

Parzival-05 force-pushed the david/new-validation-design branch 2 times, most recently from b24850b to 9ef0dca Compare December 30, 2024 06:19

Parzival-05 force-pushed the david/model-validation branch from bb2a8ea to fc3ecf9 Compare December 30, 2024 06:21

Parzival-05 force-pushed the david/new-validation-design branch from 9ef0dca to 7657a7b Compare December 30, 2024 17:48

Parzival-05 force-pushed the david/model-validation branch from fc3ecf9 to cd56d10 Compare December 30, 2024 17:52

Parzival-05 marked this pull request as draft January 8, 2025 22:07

Parzival-05 force-pushed the david/model-validation branch from cd56d10 to 0a0a262 Compare January 31, 2025 21:24

emnigma reviewed Feb 6, 2025

View reviewed changes

Parzival-05 force-pushed the david/model-validation branch from 3b8cedc to d345340 Compare February 11, 2025 07:20

Parzival-05 added 16 commits March 7, 2025 16:36

Add path resolving

6cd7b1c

More secure deletion

805fc25

refactor: Rename

1859ae8

perf: Use list comprehension instead

5f92e5b

Use entrypoint of onnx converter

21811b1

Delete svms output path only if exists

a390a42

Limit the number of steps

bce8a9d

Better logging && log and save result in case of full coverage by def…

dbb25fa

…ault searcher

Add logging in case of non-100% coverage stopping

b33eec6

Format process output

6e92d21

Don't log at 100% coverage

e5f8338

Implement a new process for saving steps in validation with model

b814b69

Add a call if need to save the steps

543d655

Update VSharp

0061d7c

Remove old files

63fd379

Code review suggestions

3fe73d1

Parzival-05 force-pushed the david/model-validation branch from 576996c to 3fe73d1 Compare March 7, 2025 13:39

Parzival-05 added 2 commits March 9, 2025 12:31

Better docs & naming for are_steps_required

10f17de

Reuse result of is_update_map_required

9e03492

Parzival-05 marked this pull request as ready for review March 9, 2025 10:09

Parzival-05 mentioned this pull request Mar 9, 2025

Refactor of actions #242

Open

Parzival-05 requested review from emnigma and Anya497 March 9, 2025 11:29

gsvgit requested changes Mar 13, 2025

View reviewed changes

Parzival-05 added 2 commits March 15, 2025 23:04

Add type annotations

bd9462a

Move get_hetero_data to dataset

7176048

Parzival-05 requested a review from gsvgit March 15, 2025 20:09

Parzival-05 added 2 commits March 16, 2025 00:35

Refactor

646a8ea

Make port fields non optional

6479a5a

emnigma reviewed Mar 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New validation mode: via sending model #205

New validation mode: via sending model #205

Parzival-05 commented Dec 27, 2024

gsvgit Mar 13, 2025

Parzival-05 Mar 15, 2025

gsvgit Mar 13, 2025

gsvgit Mar 13, 2025

gsvgit Mar 13, 2025

Parzival-05 Mar 15, 2025

gsvgit Mar 13, 2025

Parzival-05 Mar 15, 2025

emnigma Mar 17, 2025

gsvgit Mar 13, 2025

Parzival-05 Mar 15, 2025

gsvgit Mar 13, 2025

Parzival-05 Mar 15, 2025

emnigma left a comment

emnigma Mar 17, 2025

emnigma Mar 17, 2025

emnigma Mar 17, 2025

emnigma Mar 17, 2025

emnigma Mar 17, 2025

	from onyx import entrypoint, load_gamestate, resolve_import_model
	from onyx import entrypoint as save_torch_model_to_onnx_file, load_gamestate, resolve_import_model

New validation mode: via sending model #205

Are you sure you want to change the base?

New validation mode: via sending model #205

Conversation

Parzival-05 commented Dec 27, 2024

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

emnigma 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