Conversation
…plementation, update README and requirements
There was a problem hiding this comment.
Pull Request Overview
This PR implements a simplified version of the ConSpec algorithm, adding evaluation capabilities and improving the overall training framework. The changes focus on creating separate training and evaluation scripts, enhancing data collection and visualization, and updating configuration parameters.
Key changes include:
- Separation of training and evaluation workflows with dedicated scripts
- Addition of data collection and visualization utilities
- Configuration updates and code organization improvements
Reviewed Changes
Copilot reviewed 123 out of 299 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| run_eval.sh | SLURM script for running ConSpec evaluation jobs |
| run.sh | SLURM script for running ConSpec training jobs |
| main_eval.py | New evaluation script with checkpoint loading and data saving |
| main.py | New training script with checkpoint saving and improved logging |
| misc.py | Added visualization functions and updated wandb entity |
| data_collection.py | Utilities for collecting and processing cosine similarity data |
| analysis.py | Analysis and visualization tools for prototype states |
| collect_data.py | Data collection and CSV processing utilities |
| misc_util.py | Added object serialization utility |
| Conspec/ | Updates to ConSpec implementation including improved prototype handling |
| a2c_ppo_acktr/ | Configuration updates and code cleanup |
Comments suppressed due to low confidence (1)
a2c_ppo_acktr/storage.py:1
- Functions marked as non-working should either be fixed or removed. Having broken functions in the codebase can lead to confusion and potential bugs if accidentally used.
import torch
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def serialize_object(obj): | ||
| return {attr: getattr(obj, attr) for attr in dir(obj) if not attr.startswith('__') and not callable(getattr(obj, attr))} |
There was a problem hiding this comment.
This function is duplicated in both misc_util.py and misc.py. Consider consolidating to avoid code duplication and maintain consistency.
| if "/" in exp_suffix: | ||
| exp_suffix = "_".join(exp_suffix.split("/")[:-1]) | ||
| wandb.init(entity="sunchipsster", project=wandb_project_name, name=exp_name + "_" + exp_suffix, tags=wandb_tags, reinit=True) | ||
| wandb.init(entity="mandanasmi", project=wandb_project_name, name=exp_name + "_" + exp_suffix, tags=wandb_tags, reinit=True) |
There was a problem hiding this comment.
The wandb entity should be configurable rather than hardcoded. Consider making this a parameter or environment variable to improve flexibility across different users and deployments.
| pdb.set_trace() | ||
|
|
There was a problem hiding this comment.
Debug breakpoint should be removed from production code. This will cause the program to halt execution when reached.
| pdb.set_trace() |
| plt.savefig('observation.png') | ||
| wandb.log({"observation Plot": wandb.Image('observation.png')}) |
There was a problem hiding this comment.
The hardcoded filename 'observation.png' could cause conflicts in concurrent executions. Consider using unique filenames or temporary files.
| num_updates = int(args.num_env_steps) // args.num_steps // args.num_processes | ||
| logger.start() | ||
|
|
||
| checkpoint = torch.load('data/kd3/checkpoint_epoch_1999.pth') |
There was a problem hiding this comment.
The checkpoint path is hardcoded, making the evaluation script inflexible. Consider adding this as a command-line argument to make the script more reusable across different experiments.
No description provided.