Implement a benchmarking mode #252

Adamantios · 2024-04-24T15:37:23Z

No description provided.

Also adds the `check_stop_trading_abci` to the `testenv` in `tox.ini`.

Pinned because of safety's reported vulnerability with id 64227. The new `tomte` version addresses this as its lockfile has set `jinja2 ==3.1.3`.

Continues work on 128db35.

This reverts commit 4c9e42e.

Fixes the issue with cross-period persisted keys not being available after a reset when using the benchmarking mode. This change should ideally be moved to the core to avoid hacking around here. Extending the `setup` method with this functionality can be the solution in valory-xyz/open-autonomy#2131.

Performs the change intended in 4c9e42e.

Adamantios · 2024-05-09T14:56:43Z

In order to run the mocked trader's version the following environment variables need to be set:

BENCHMARKING_MODE_ENABLED -> true
BENCHMARKING_MODE_DATASET_FILENAME -> the value has to be the filename of the input dataset, which must be present under the STORE_PATH folder of the agent.

Other variables can also be set - you may find them under the benchmarking_mode - in order to change the behaviour of the mocked agent.

Optionally, you may also set the RESET_PAUSE_DURATION to a smaller amount so that the mocked version runs faster.

All these environment variables can be set in the quickstart script to simplify the running process.

cyberosa

Well done.

cyberosa · 2024-05-09T15:14:09Z

packages/valory/agents/trader/aea-config.yaml

+    args:
+      enabled: ${bool:false}
+      native_balance: ${int:10000000000000000000}
+      collateral_balance: ${int:10000000000000000000}


Why int instead of float? I heard that the bet amounts can be very low and may include decimals

These are in WEI.

cyberosa · 2024-05-09T15:20:41Z

packages/valory/skills/decision_maker_abci/behaviours/base.py

+            return
+
+        add_headers = False
+        results_path = self.params.store_path / self.benchmarking_mode.results_filename


I wonder if you prefer to have a separate folder called "benchmarking_results" in order not to be mixed with other data you have in that store folder...

That would require changes in the quickstart. Let's keep it simple for now.

cyberosa · 2024-05-09T15:23:32Z

packages/valory/skills/decision_maker_abci/behaviours/base.py

+            )
+            results_text = tuple(str(res) for res in results)
+            row = ",".join(results_text) + NEW_LINE
+            results_file.write(row)


where is the results_file.close()

It's not necessary, as a context manager is used.

cyberosa · 2024-05-09T15:24:43Z

packages/valory/skills/decision_maker_abci/behaviours/bet_placement.py

@@ -204,9 +204,17 @@ def _prepare_safe_tx(self) -> Generator[None, None, Optional[str]]:

    def async_act(self) -> Generator:
        """Do the action."""
+        agent = self.context.agent_address


dont you prefer to call the variable agent_address?

cyberosa · 2024-05-09T15:34:22Z

packages/valory/skills/decision_maker_abci/behaviours/decision_receive.py

+            if mode.part_prefix_mode:
+                fields[prediction_attribute] = row[field_part + mech_tool]
+            else:
+                fields[prediction_attribute] = row[mech_tool + field_part]


why this alternative? I thought the input files should be consistent with the format of the column names...

This was developed before agreeing on the column names etc. It provides flexibility so that we do not need to perform code changes if the input format changes.

cyberosa · 2024-05-09T15:40:37Z

packages/valory/skills/decision_maker_abci/behaviours/decision_request.py

@@ -75,9 +75,13 @@ def async_act(self) -> Generator:
        """Do the action."""
        with self.context.benchmark_tool.measure(self.behaviour_id).local():
            payload_content = None
+            mocking_mode: Optional[bool] = self.benchmarking_mode.enabled


is the mocking_mode only active for the benchmarking_mode? Then why not giving the same name?

🤷‍♂️

cyberosa · 2024-05-09T15:45:06Z

packages/valory/skills/decision_maker_abci/fsm_specification.yaml

    (DecisionReceiveRound, DONE): BetPlacementRound
    (DecisionReceiveRound, MECH_RESPONSE_ERROR): BlacklistingRound
    (DecisionReceiveRound, NO_MAJORITY): DecisionReceiveRound
    (DecisionReceiveRound, ROUND_TIMEOUT): DecisionReceiveRound
    (DecisionReceiveRound, TIE): BlacklistingRound
    (DecisionReceiveRound, UNPROFITABLE): BlacklistingRound
    (DecisionRequestRound, DONE): FinishedDecisionRequestRound
-    (DecisionRequestRound, NONE): ImpossibleRound


So the NONE transition is not possible anymore?

It was never possible, check the comment:

trader/packages/valory/skills/decision_maker_abci/rounds.py

Lines 207 to 208 in da93b1a

# this is here because of `autonomy analyse fsm-specs` falsely reporting it as missing from the transition

Event.NONE: ImpossibleRound,

cyberosa · 2024-05-09T15:46:32Z

packages/valory/skills/decision_maker_abci/models.py

+            "confidence_field_part", kwargs, str
+        )
+        # this is the mode for the p and confidence parts
+        # if the flag is `True`, then the field parts are used as prefixes, otherwise as suffixes


I prefer to keep only one format, because I need to process this files also for the evaluation part and I prefer to be consistent with column names.

Ok, it does not depend on the code here though, it depends on the input data.

Adamantios added the enhancement New feature or request label Apr 24, 2024

Adamantios force-pushed the feat/benchmarking-mode branch from 73b601a to 3f95295 Compare April 24, 2024 15:42

Adamantios added 8 commits April 26, 2024 19:07

feat: implement benchmarking mode

e7de78a

chore: fix linting issues

bd6e4bf

Also adds the `check_stop_trading_abci` to the `testenv` in `tox.ini`.

fix: address the issue reported by safety with id 64227

bd4c324

Pinned because of safety's reported vulnerability with id 64227. The new `tomte` version addresses this as its lockfile has set `jinja2 ==3.1.3`.

fix: add missing dependency to the agent

a8c7243

chore: fix the copyright commands

6eaf05c

fix: update the link to the mint page

553c904

chore: add type annotation required by mypy

71a6388

chore: run generators

ea9e901

Adamantios force-pushed the feat/benchmarking-mode branch from 3f95295 to ea9e901 Compare April 26, 2024 16:12

cyberosa and others added 15 commits May 7, 2024 15:14

BENCHMARKING_ENABLED then skip Sampling and go to ToolSelectionRound

4c9e42e

adding option to get tool names from csv file in benchmarking mode

128db35

fix: retrieve benchmark tools at the correct point

9ad5a4d

Continues work on 128db35.

fix: simplify and remove the pandas dependency

c2ec19c

Continues work on 128db35.

fix: revert changes to the fsm specs

46d1a77

This reverts commit 4c9e42e.

refactor: replace exit with sys.exit

79c6d74

feat: skip the sampling round when benchmarking is enabled

34229d2

Performs the change intended in 4c9e42e.

fix: use a mocked bet on benchmarking mode

aca7cd7

refactor: use the benchmarking_mode parameters instead

81dab44

fix: collateral amount calculation in benchmarking mode

f6321ff

chore: run generators

a6e4189

fix: preserve the initial format of the questions

7d973af

refactor: add the prefix to the benchmarking mode's env vars

d34d62e

chore: run generators

5902f33

Adamantios changed the title ~~[WIP]: Implement a benchmarking mode~~ Implement a benchmarking mode May 9, 2024

Adamantios marked this pull request as ready for review May 9, 2024 14:56

Adamantios requested a review from 0xArdi May 9, 2024 14:57

Adamantios requested a review from jmoreira-valory May 9, 2024 14:57

jmoreira-valory approved these changes May 9, 2024

View reviewed changes

dvilelaf approved these changes May 9, 2024

View reviewed changes

Adamantios merged commit 018d02f into main May 9, 2024
6 checks passed

Adamantios deleted the feat/benchmarking-mode branch May 9, 2024 15:16

cyberosa approved these changes May 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement a benchmarking mode #252

Implement a benchmarking mode #252

Adamantios commented Apr 24, 2024

Adamantios commented May 9, 2024

cyberosa left a comment

cyberosa May 9, 2024

Adamantios May 10, 2024

cyberosa May 9, 2024

Adamantios May 10, 2024

cyberosa May 9, 2024

Adamantios May 10, 2024

cyberosa May 9, 2024

cyberosa May 9, 2024

Adamantios May 10, 2024

cyberosa May 9, 2024

Adamantios May 10, 2024

cyberosa May 9, 2024

Adamantios May 10, 2024

cyberosa May 9, 2024

Adamantios May 10, 2024

	# this is here because of `autonomy analyse fsm-specs` falsely reporting it as missing from the transition
	Event.NONE: ImpossibleRound,

Implement a benchmarking mode #252

Implement a benchmarking mode #252

Conversation

Adamantios commented Apr 24, 2024

Adamantios commented May 9, 2024

cyberosa 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment