Closed josephdviviano closed 6 months ago
FYI @josephdviviano I changed the "base" branch to rethinking_sampling instead of master. This allows us to view this PRs changes in isolation. When you merge #147, this PR will automatically update to be based off of master again! Alternatively, you can merge this PR into #147 and then merge #147 into master and it will have the same effect. I would suggest merging #147 first though and then iterating on / merging this PR in isolation 🙌
I implemented the renaming and also realized I needed to update the documentation which is now fixed.
To be merged after #147
make_States_class
andmake_Actions_class
no longer need to be defined by the user - all relevant logic is submitted directly to theEnv
orDiscreteEnv
subclass. (Of course, the user could overwrite the defaultDefaultEnvState
andDefaultEnvAction
classes returned bymake_States_class
andmake_Actions_class
IIF they require boutique functionality, but this is not expected to be a normal workflow).maskless_
naming fromstep
andbackward_step
.As a result of this, multiple methods are offloaded from the
States
class into theEnv
, andmake_random_states_tensor
must be passed from theEnv
to theStates
class, which accounts for a large number of these diffs.As an example, see the below Env definition for the line environment, which is complete: