Skip to content

Conversation

@shuo-yuan
Copy link
Contributor

No description provided.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The code changes include adding a bash script for Tinker RL training for the MemAgent task in the OSWorld environment and a YAML configuration file for the OSWorld task. The review comments point out the hardcoded absolute paths for datasets and the output directory in the bash script, suggesting the use of environment variables with sensible defaults to make the script more portable.

Comment on lines +14 to +20
DATASET_FILE="/home/ubuntu/shuo/osworld/OSWorld_llm_agentsynth/osworld_train_8.parquet"

EVAL_DATASET_FILE="/home/ubuntu/shuo/osworld/OSWorld_llm_agentsynth/osworld_train_8.parquet"

# Output directory
NAME="${NAME:-jan03_qwen3_8b_osworld_tinker_lr4e_5_rank128}"
OUTPUT_DIR="/home/ubuntu/shuo/osworld/checkpoints/${NAME}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The script contains hardcoded absolute paths for datasets and the output directory, which include a specific user's home directory (/home/ubuntu/shuo). This makes the script not portable and difficult for other users to run without modification.

It's a best practice to avoid hardcoding user-specific paths. You can make these paths configurable, for example, by using environment variables with sensible defaults, similar to how other variables like MODEL_NAME are handled in this script.

Suggested change
DATASET_FILE="/home/ubuntu/shuo/osworld/OSWorld_llm_agentsynth/osworld_train_8.parquet"
EVAL_DATASET_FILE="/home/ubuntu/shuo/osworld/OSWorld_llm_agentsynth/osworld_train_8.parquet"
# Output directory
NAME="${NAME:-jan03_qwen3_8b_osworld_tinker_lr4e_5_rank128}"
OUTPUT_DIR="/home/ubuntu/shuo/osworld/checkpoints/${NAME}"
DATASET_FILE="${DATASET_FILE:-/path/to/your/osworld_train_8.parquet}"
EVAL_DATASET_FILE="${EVAL_DATASET_FILE:-/path/to/your/osworld_train_8.parquet}"
# Output directory
NAME="${NAME:-jan03_qwen3_8b_osworld_tinker_lr4e_5_rank128}"
OUTPUT_DIR="${OUTPUT_DIR:-/path/to/your/checkpoints/${NAME}}"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant