Refactor: Agent Class #343
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rove readability ## Agent Class Constructor Refactor ### Overview This update refactors the `Agent` class constructor by introducing default values for the parameters `min_observations` and `observations_per_step`. This change aims to improve code clarity, simplify internal logic, and reduce potential errors arising from `None` values. ### Changes - **`min_observations`**: - **Before**: Could be `None`. - **After**: Defaults to `0`. This means the agent can start learner steps immediately unless otherwise specified. - **`observations_per_step`**: - **Before**: Could be `None`. - **After**: Defaults to `1.0`, indicating one learner step per actor observation by default. ### Benefits - Removes the need for repeated `None` checks in the code. - Simplifies the update logic and makes it more readable. - Provides a clear, sensible default behavior for agent training schedules. --- This refactor improves maintainability and reduces potential bugs caused by unhandled `None` values, helping the project stay robust and easier to extend.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This refactor of the Agent class focuses on code clarity and maintainability while explicitly ensuring that no original behavior is changed.
It merges two key efforts:
A readability-driven update that clarified constructor logic and internal structure.
A behavioral reversion that restores original semantics involving optional parameters and constants.
Behavior Preservation
Removed default values for:
min_observations: Restored to allow None (was temporarily defaulted to 0).
observations_per_step: Restored to allow None (was temporarily defaulted to 1.0).
These changes ensure the original learning schedule logic remains untouched, especially where conditional logic relies on None.
Scope Reversion
Reverted _MAX_BATCH_SIZE_UPPER_BOUND:
Previously extracted as a global constant.
Now returned to inline usage within the constructor to avoid unnecessary scope expansion or global leakage.
Additional Improvements
Even while reverting the above behavioral changes, we retained the following structural refactors for clarity:
Clearer method and argument documentation.
Renamed internal variables for expressiveness (steps_since_min, update_actor, etc.).
Improved inline comments to better explain conditions and edge cases.
Clean, consistent formatting throughout the class.