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

Related to Fixing errors with _obtain_steps function #27 #28

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

alexofficial
Copy link

@alexofficial alexofficial commented Mar 6, 2024

Type of change

  • Bugfix

Description

First change - Fix a TypeError caused by an unenumerable object

  • What has been changed?: The TypeError caused by an unenumerable object when using a for loop with rollout_lengths[i] has been fixed. Instead of directly accessing rollout_lengths[i], the fix involves iterating directly over the range of rollout_lengths[i].

  • How the logic works?: The previous implementation attempted to use rollout_lengths[i] directly in a loop, which caused a TypeError due to rollout_lengths being an unenumerable object created with self.random_.choice(), resulting in a numpy.int32 object. The fix involves iterating over the range of rollout_lengths[i], ensuring that each step of the loop corresponds to an index within the range of rollout_lengths[i], thus avoiding the TypeError.

Second change - Fixed form params in np.zeros for action arrays :

  • The shape argument in np.zeros calls for 'actions' and 'action_probs' arrays has been corrected from one-dimensional to tuple format.
  • This change ensures that the arrays are correctly initialized with the intended dimensions (n_trajectories * step_per_trajectory, action_dim), which is the expected structure for processing action data. provide me with this format

References

Checklist

  • pass unit test (or unnecessary)
  • no errors on newly made test cases
  • no errors on existing test cases
  • applied black formatter
  • No errors on flake8
  • no warnings

Comments

  • Error with flake8: There is another error with flake, but I did not fix it because it was related to MultipleLoggedDataset, which I have not try to train yet (I left it for future work) - synthetic.py:1290:25: F821 undefined name 'logged_dataset_'

  • black format removes the () for the np.zero: I suspect that the wrong dimensions are due to the Black format. When I apply Black to synthetic.py, for some reason it removes the () in the lines, which I have already corrected. I assume Black was applied throughout the directory, so the changes were just picked up. Be aware of this 💯

Patsanis added 3 commits March 6, 2024 17:13
The shape argument in np.zeros calls for 'actions' and 'action_probs'
arrays has been corrected from one-dimensional to tuple format.

This change ensures that the arrays are correctly initialized with the
intended dimensions (n_trajectories * step_per_trajectory, action_dim),
which is the expected structure for processing action data.
The error occurred when the for loop was used with `rollout_lengths[i]`,
where `rollout_lengths` was created with `self.random_.choice()`,
resulting in a `numpy.int32` object.

The problem has been fixed by iterating directly over the range of `rollout_lengths[i]` instead.

Fixing errors with _obtain_steps function hakuhodo-technologies#27
Adding F821 undefined name 'next_state' based on Flask8.

There is also another: synthetic.py:1289:25: F821 undefined name
'logged_dataset_'. However, as this is with MultipleLoggedDataset
and it was not tested, I have left it for future work.
@alexofficial alexofficial changed the title Related to ixing errors with _obtain_steps function #27 Related to Fixing errors with _obtain_steps function #27 Mar 6, 2024
self.behavior_policy existis only in this point and it is uncessary. Rename the variable self.behavior_policy to just behavior_policy will work.
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.

1 participant