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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve rollout utility #106

Closed
neo-alex opened this issue Nov 19, 2021 · 0 comments 路 Fixed by #367
Closed

Improve rollout utility #106

neo-alex opened this issue Nov 19, 2021 · 0 comments 路 Fixed by #367

Comments

@neo-alex
Copy link
Collaborator

馃殌 Feature

Fix, merge and improve rollout utilities

Pitch

There are currently 2 almost identical rollout utilities in utils.py (rollout and rollout_episode) that could be improved & merged into 1, which would avoid code duplication and be less confusing for end-users.

I propose following changes:

  • remove rollout_episode but add a boolean return_episodes parameter to rollout to optionally enable the same feature (without current bugs, namely when from_state is None or when num_episodes > 1)
  • clean save_file feature by introducing a file_formatter parameter enabling customisation of file content from collected episodes (alternatively, we could try to solve the 2 first points jointly e.g. by introducing episodes_formatter which, if not None, would collect episodes in a certain format and either return them or save them to file given the save_file parameter)
  • possibly add callbacks (e.g. step_callback & episode_callback) to give the end-user the option to add custom code within the rollout loops
nhuet added a commit to nhuet/scikit-decide that referenced this issue May 21, 2024
@nhuet
Unify the rollout utilities
d943079 
This fixes airbus#106

We merge rollout and rollout_episode together:
- add a return_episodes boolean arg to rollout, deciding wether to
  return episodes
- episodes are returned as a list of episodes, each episode being a
  tuple of observations, actions, and values
  (previously returned prematurely after one episode only one tuple even
  if num_episodes was >1)
- update previous code using rollout_episode, by using rollout with
  return_episodes=True, and using the first episode of the list

Fix verbose behaviour by setting back logger level to previous level at
the end of the rollout. (Previously was setting once for all the logger
level to debug, even when going out of rollout.)
nhuet added a commit to nhuet/scikit-decide that referenced this issue May 21, 2024
@nhuet
Unify the rollout utilities
bf16bca 
This fixes airbus#106

We merge rollout and rollout_episode together:
- add a return_episodes boolean arg to rollout, deciding wether to
  return episodes
- episodes are returned as a list of episodes, each episode being a
  tuple of observations, actions, and values
  (previously returned prematurely after one episode only one tuple even
  if num_episodes was >1)
- update previous code using rollout_episode, by using rollout with
  return_episodes=True, and using the first episode of the list

Fix verbose behaviour by setting back logger level to previous level at
the end of the rollout. (Previously was setting once for all the logger
level to debug, even when going out of rollout.)
@nhuet nhuet mentioned this issue May 21, 2024
Merged
nhuet added a commit to nhuet/scikit-decide that referenced this issue May 21, 2024
@nhuet
Unify the rollout utilities
5946dbe 
This fixes airbus#106

We merge rollout and rollout_episode together:
- add a return_episodes boolean arg to rollout, deciding wether to
  return episodes
- episodes are returned as a list of episodes, each episode being a
  tuple of observations, actions, and values
  (previously returned prematurely after one episode only one tuple even
  if num_episodes was >1)
- in rollout_episode, verbose=False was muting the logger.info("goal
  reached ..."), instead we introduce a parameter to change the level of
  this logging. So that in particular in MetaPolicy, we can relegate it
  at debug level.
- update previous code using rollout_episode, by using rollout with
  return_episodes=True, and using the first episode of the list

Fix verbose behaviour by setting back logger level to previous level at
the end of the rollout. (Previously was setting once for all the logger
level to debug, even when going out of rollout.)
g-poveda pushed a commit that referenced this issue May 21, 2024
@nhuet @g-poveda
Unify the rollout utilities
338c831 
This fixes #106

We merge rollout and rollout_episode together:
- add a return_episodes boolean arg to rollout, deciding wether to
  return episodes
- episodes are returned as a list of episodes, each episode being a
  tuple of observations, actions, and values
  (previously returned prematurely after one episode only one tuple even
  if num_episodes was >1)
- in rollout_episode, verbose=False was muting the logger.info("goal
  reached ..."), instead we introduce a parameter to change the level of
  this logging. So that in particular in MetaPolicy, we can relegate it
  at debug level.
- update previous code using rollout_episode, by using rollout with
  return_episodes=True, and using the first episode of the list

Fix verbose behaviour by setting back logger level to previous level at
the end of the rollout. (Previously was setting once for all the logger
level to debug, even when going out of rollout.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Unify the rollout utilities nhuet/scikit-decide
1 participant
@neo-alex