Update for lang-graph rewrite (maltoolbox 0.2) by nkakouros · Pull Request #89 · mal-lang/mal-simulator

nkakouros · 2025-01-13T18:17:59Z

Fixes to make the simulator work with the lang-graph rewrite of mal-toolbox.

mrkickling · 2025-01-27T10:03:19Z

Did I do double work again (#94)..

But some notes:

Your solution is mostly cleaner than mine
- But I think the association/step types would need to be unique (set instead of list).
I am a bit unsure about the class.name.removeprefix, we should have a cleaner way to fetch the full_name.. My solution seems safer but also uglier. So I am not sure about that.
The observation space also needs to be updated (part of my PR)
The model files need to be updated (part of my PR)

I could make a PR into your PR, keeping the parts I like in your solution but adding the required changes that are only in mine, so it is all in one place?

nkakouros · 2025-01-27T10:09:20Z

Where can i find your solution?

nkakouros · 2025-01-27T10:09:29Z

Where can I learn to read?

mrkickling · 2025-01-27T10:19:24Z

Where can I learn to read?

Well, I might also take that course so I read the name of your PR next time before I make my own:)

nkakouros · 2025-01-27T10:31:11Z

  * But I think the association/step types would need to be unique (set instead of list).

I agree.

* I am a bit unsure about the **class**.**name**.removeprefix, we should have a cleaner way to fetch the full_name.. My solution seems safer but also uglier. So I am not sure about that.

Actually is this still needed? I don't think Association_ is being prefixed anymore, which means that this can be simplified to just assoc.__class__.__name__ which is the idiomatic way of getting the class name which is also what we need (as long as we still use a model asset factory).

* The observation space also needs to be updated (part of my PR)

Can you pull that part here? You can do git checkout -p <your-branch> -- path/to/file/Is/it/model.py?

* The model files need to be updated (part of my PR)

Can you pull these changes here?

I could make a PR into your PR, keeping the parts I like in your solution but adding the required changes that are only in mine, so it is all in one place?

Yes, makes sense. I think you can push directly here.

mrkickling · 2025-01-27T10:35:25Z

Actually is this still needed? I don't think Association_ is being prefixed anymore, which means that this can be simplified to just assoc.__class__.__name__ which is the idiomatic way of getting the class name which is also what we need (as long as we still use a model asset factory).

I think it is still prepends 'Association_' in the lang classes factory. But that will soon be forgotten.

I will try to put the valid changes from my PR into this PR.

mrkickling · 2025-01-27T12:05:58Z

Let me know what you think

nkakouros · 2025-01-27T12:20:39Z

        self._index_to_id = [n.id for n in self.attack_graph.nodes]
-        self._index_to_full_name = [n.full_name
-                                    for n in self.attack_graph.nodes]
-        self._index_to_asset_type = [n.name for n in self.lang_graph.assets]
-        self._index_to_step_name = [n.asset.name + ":" + n.name
-                                    for n in self.lang_graph.attack_steps]
-        self._index_to_model_asset_id = [int(asset.id) for asset in \
-            self.attack_graph.model.assets]
-        self._index_to_model_assoc_type = [assoc.name + '_' + \
-            assoc.left_field.asset.name + '_' + \
-            assoc.right_field.asset.name \
-                for assoc in self.lang_graph.associations]
+        self._index_to_full_name = (
+            [n.full_name for n in self.attack_graph.nodes]
+        )
+        self._index_to_asset_type = (
+            [n.name for n in self.lang_graph.assets.values()]
+        )
+
+        unique_step_type_names = {
+            n.full_name
+            for asset in self.lang_graph.assets.values()
+            for n in asset.attack_steps.values()
+        }
+        self._index_to_step_name = list(unique_step_type_names)
+
+        self._index_to_model_asset_id = (
+            [int(asset.id) for asset in self.attack_graph.model.assets]
+        )
+
+        unique_assoc_type_names = {
+            assoc.full_name
+            for asset in self.lang_graph.assets.values()
+            for assoc in asset.associations.values()
+        }
+        self._index_to_model_assoc_type = list(unique_assoc_type_names)

        # Lookup dicts attribute to index
        self._id_to_index = {


Most of these will be possible to drop once mal-lang/mal-toolbox#105 is merged.

…ction order

…step names

nkakouros requested a review from andrewbwm January 13, 2025 18:17

nkakouros force-pushed the lang-graph-rewrite branch from 643decd to d34c367 Compare January 16, 2025 17:32

andrewbwm assigned mrkickling Jan 23, 2025

mrkickling changed the title ~~Update for lang-graph rewrite~~ Update for lang-graph rewrite (maltoolbox 0.2) Jan 27, 2025

nkakouros commented Jan 27, 2025

View reviewed changes

Comment thread malsim/scenario.py Outdated

nkakouros commented Jan 27, 2025

View reviewed changes

Comment thread malsim/sims/mal_simulator.py Outdated

nkakouros commented Jan 27, 2025

View reviewed changes

Nikolaos Kakouros and others added 9 commits January 27, 2025 15:15

Update for lang-graph rewrite

dc123a7

Upgrade models to new model format

9ab7b61

Use names instead of ids in scenario example test and update to new a…

9942b36

…ction order

Adapt scenario validation with new lang graph

44b76c6

Use mal-toolbox 0.2

66e2256

Adapt mal simulator to the new lang graph

49e2080

Use set of hashable attack steps to count number of steps instead of …

33409cb

…step names

Variable renaming step->step_name

5bdbc92

Simplify set add expression to use |=

8601ab3

mrkickling force-pushed the lang-graph-rewrite branch from 12e8b77 to 8601ab3 Compare January 27, 2025 14:16

mrkickling self-requested a review January 27, 2025 14:17

mrkickling approved these changes Jan 27, 2025

View reviewed changes

andrewbwm approved these changes Jan 27, 2025

View reviewed changes

mrkickling merged commit 111b572 into main Jan 27, 2025

nkakouros deleted the lang-graph-rewrite branch January 29, 2025 15:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update for lang-graph rewrite (maltoolbox 0.2)#89

Update for lang-graph rewrite (maltoolbox 0.2)#89
mrkickling merged 9 commits into
mainfrom
lang-graph-rewrite

nkakouros commented Jan 13, 2025

Uh oh!

mrkickling commented Jan 27, 2025 •

edited

Loading

Uh oh!

nkakouros commented Jan 27, 2025

Uh oh!

nkakouros commented Jan 27, 2025

Uh oh!

mrkickling commented Jan 27, 2025

Uh oh!

nkakouros commented Jan 27, 2025

Uh oh!

mrkickling commented Jan 27, 2025

Uh oh!

mrkickling commented Jan 27, 2025

Uh oh!

Uh oh!

Uh oh!

nkakouros Jan 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

nkakouros commented Jan 13, 2025

Uh oh!

mrkickling commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nkakouros commented Jan 27, 2025

Uh oh!

nkakouros commented Jan 27, 2025

Uh oh!

mrkickling commented Jan 27, 2025

Uh oh!

nkakouros commented Jan 27, 2025

Uh oh!

mrkickling commented Jan 27, 2025

Uh oh!

mrkickling commented Jan 27, 2025

Uh oh!

Uh oh!

Uh oh!

nkakouros Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mrkickling commented Jan 27, 2025 •

edited

Loading