Closed KilianFt closed 1 year ago
@CodiumAI-Agent /review
๐ฏ Main theme: Refactoring and enhancement of the codebase to support new user types and improve the model training process.
๐ PR summary: This PR introduces significant changes to the codebase, including the addition of new user classes (FrankensteinProportionalUser, EMGProportionalUser), the implementation of a new SLController, and modifications to the environment and main script to accommodate these changes. The PR also includes the addition of a new dataset (NinaPro1) and a script for real-time prediction testing.
๐ Type of PR: Enhancement
๐งช Relevant tests added: No
๐ Security concerns: No security concerns found
๐ก General suggestions: The PR introduces several new features and improvements, which are generally well-implemented. However, there are a few areas that could benefit from further refinement. Firstly, it would be beneficial to add some tests to ensure the new features work as expected. Secondly, the code could be made more modular by separating different functionalities into distinct functions or classes. Lastly, it's recommended to handle exceptions and errors more gracefully, especially for the new user classes and the real-time prediction script.
๐ค Code feedback:
relevant file: users.py
suggestion: Consider adding error handling for the case when the user action is zero in the 'action_to_features' method of the 'FrankensteinProportionalUser' class. This will prevent the program from crashing and provide a more informative error message. [important]
relevant line: '+ raise ValueError("User action is zero!")'
relevant file: realtime_pred_test.py
suggestion: It's recommended to add exception handling for the case when the queue is empty in the main loop. This will prevent the program from crashing if the queue is empty. [important]
relevant line: '+ emg = list(q.get())'
relevant file: main.py
suggestion: The 'onehot_to_dof' function could be made more efficient by using a numpy array for 'dof_cmd' instead of a list. This would also make the code more consistent, as numpy arrays are used elsewhere in the code. [medium]
relevant line: '+ dof_cmd = np.zeros(2)'
relevant file: controllers.py
suggestion: It's recommended to add a check to ensure that the 'states' and 'optimal_actions' tensors are on the same device before calculating the loss in the 'sl_update' method of the 'SLController' class. This will prevent potential errors due to mismatched devices. [medium]
relevant line: '+ loss = self.criterion(predicted_action, target_action)'
Tag me in a comment '@CodiumAI-Agent' and add one of the following commands: /review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option. /describe: Modify the PR title and description based on the contents of the PR. /improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback. /ask \<QUESTION>: Pose a question about the PR. /update_changelog: Update the changelog based on the PR's contents.
To edit any configuration parameter from configuration.toml, add --config_path=new_value For example: /review --pr_reviewer.extra_instructions="focus on the file: ..." To list the possible configuration parameters, use the /config command.
things to do