Conversation
alexcmtk
left a comment
alexcmtk
left a comment
There was a problem hiding this comment.
Small changes. I'll review again once done.
| # option (not recommended) you can uncomment the following to ignore the entire idea folder. | ||
| #.idea/ | ||
| wandb/ | ||
| *.csv |
There was a problem hiding this comment.
It may not be great to ignore all csv and text files. While, yes, we do not submit data, typically, important information can be in this format sometimes. The preferred pattern is to ignore all txt, csv files in some nominated subfolder (e.g. where you keep your logs).
| state = np.array([state]) | ||
| elif isinstance(state, np.ndarray): | ||
| # Ensure the state is a numpy array | ||
| state = state |
|
|
||
| if isinstance(state, dict): | ||
| # Extract the observation from the dictionary | ||
| state = state['observation'] |
There was a problem hiding this comment.
All other returns are np.ndarray but this doesn't have to be? return type is inconsistent.
| self.action_transformation = action_transformation | ||
| # File paths | ||
| self.csv_file = 'krvi_metrics.csv' | ||
| self.config_file = 'config.txt' |
There was a problem hiding this comment.
If this is the txt file you're ignoring then it shouldn't be ignored.
| self.verbose = verbose | ||
| self.seed = seed | ||
| self.optim_botorch = optim_botorch | ||
| self.optimal_V = optimal_V |
There was a problem hiding this comment.
I understand this is optional but if it's only used for computing regret, it should not be here
| def train(self, T: int): | ||
|
|
||
| action_space = np.arange(self.env.action_space.n) # Assuming discrete action space | ||
|
|
There was a problem hiding this comment.
whitespace (final time - please remove throughout)
|
|
||
| for episode in range(T): | ||
| if self.verbose > 0: | ||
| print(f'Episode {episode}') |
There was a problem hiding this comment.
It's ok to print things, but it's better to log them. On top of the CSV all std output can be captured to a file (another file).
|
|
||
|
|
||
|
|
||
| def train(self, T: int): |
There was a problem hiding this comment.
This method is very long. It's usually a bad pattern. Can you extract some private methods? A good rule of thumb is that wherever you have a comment header in the body e.g. # Execute episode you should instead have a private method self._execute_episode instead. Aim to have no more than 10 lines per method like this.
| import gymnasium as gym | ||
| from gpytorch.kernels import ScaleKernel, RBFKernel | ||
| import numpy as np | ||
| def action_transformation(action_index): |
|
|
||
| env=gym.make('FrozenLake-v1', desc=None, map_name="4x4", is_slippery=False) | ||
| env = FrozenLake2DStateWrapper(env, rescale=True) | ||
| optimal_V= None |
There was a problem hiding this comment.
Since I'm now convinced this is not needed, we shouldn't make it part of the algorithm? Which other predictive model or RL algorithm that you have seen takes the ground truth as part of the constructor, optional or not?
2 participants
Added
KRVI_algo_final.pyand Ensuredmasterremains clean by moving changes to this branch.