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.

Sign up for GitHub

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

Jump to bottom

Policy combiner #357

Merged
merged 15 commits into from
Sep 10, 2024
Merged

Policy combiner #357

merged 15 commits into from
Sep 10, 2024

Conversation

tvmarino
Copy link
Collaborator

@tvmarino tvmarino commented Sep 6, 2024

Combines two tf-agents policies with the given signature spec in get_input_signature.

@tvmarino
Combine two tf_agents policies with timestep spec
a06d452 
given by combine_tfa_policies_lib.get_input_signature()
and action spec given by combine_tfa_policies_lib.get_action_spec()
The combiner policy uses a new timestep spec feature "model_selector"
to select the requested policy at the current state. The feature is
computed as a md5 hash from the respective policies names.
@tvmarino
Merge branch 'main' of https://github.com/google/ml-compiler-opt
15876f7
mtrofin
mtrofin reviewed Sep 6, 2024
View reviewed changes
compiler_opt/tools/combine_tfa_policies_lib.py Outdated Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib.py Outdated Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies.py Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib_test.py Outdated Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib_test.py Outdated Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib_test.py Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib_test.py Outdated Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib.py Show resolved Hide resolved
mtrofin
mtrofin reviewed Sep 7, 2024
View reviewed changes
Copy link
Collaborator

@mtrofin mtrofin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good, just mostly nits

boomanaiden154
boomanaiden154 reviewed Sep 9, 2024
View reviewed changes
Copy link
Collaborator

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just nits from my end.

compiler_opt/tools/combine_tfa_policies.py Outdated Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib.py Outdated Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib_test.py Outdated Show resolved Hide resolved
@mtrofin mtrofin mentioned this pull request Sep 9, 2024
Open
boomanaiden154
boomanaiden154 approved these changes Sep 9, 2024
View reviewed changes
Copy link
Collaborator

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two nits that should be fixed before landing. Otherwise LGTM.

compiler_opt/tools/combine_tfa_policies.py Outdated Show resolved Hide resolved
compiler_opt/tools/combine_tfa_policies_lib.py Outdated Show resolved Hide resolved
@tvmarino tvmarino merged commit 2878b51 into google:main Sep 10, 2024
15 checks passed
@tvmarino tvmarino deleted the policy_combiner branch September 10, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boomanaiden154 boomanaiden154 boomanaiden154 approved these changes

@mtrofin mtrofin mtrofin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tvmarino @mtrofin @boomanaiden154