Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: faster calc_ground_truth_policy_value in SyntheticSlateBanditDataset #102

Merged
merged 15 commits into from
May 30, 2021

Conversation

aiueola
Copy link
Contributor

@aiueola aiueola commented May 30, 2021

new feature

bug fix

refactor

result

  • setting: len_list=5, n_unique_actions=10, n_rounds=1000,is_factorizable=True
  • pscore when is_factorizable=True: 1.5min
  • expected_reward_factural
    • before: standard_additive = 7.5min, cascade_additive = 7min, standard_decay = 17min, cascade_decay = 17min, independent = 17min
    • after: standard_additive = 7min, cascade_additive = 6min, standard_decay = 8.5min, cascade_decay = 6.5min, independent = 4min

test

  • add and edit corresponding tests.

others

  • minor fix on typos and docstrings

@usaito
Copy link
Contributor

usaito commented May 30, 2021

@aiueola Thanks!

[imo]

I think action_interaction_reward_function should be given action and enumerated_action separately (then, is_enumerated will be deleted). That's because enumerated_action is not sampled from a prob distribution and they are conceptually different.
https://github.com/aiueola/zr-obp/blob/b20d8158b67683c0a479ff8fe8fe6451a2a132c8/obp/dataset/synthetic_slate.py#L1171

Some minor points are below.

is_additive = reward_structure in ["standard_additive", "cascade_additive"]
is_cascade = reward_structure in ["cascade_additive", "cascade_decay"]

instead of
https://github.com/aiueola/zr-obp/blob/b20d8158b67683c0a479ff8fe8fe6451a2a132c8/obp/dataset/synthetic_slate.py#L1263

@usaito usaito merged commit 3cb08b5 into st-tech:master May 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants