Skip to content

Conversation

@as-jding
Copy link
Contributor

@as-jding as-jding commented Oct 15, 2025

Description

Added config files location reminder

Types of change

  • Bug fix πŸ›
  • New feature ✨
  • Enhancement πŸš€
  • Code refactoring πŸ”§
  • Documentation update πŸ“–
  • Chore 🧹
  • Style 🎨

Checklist

Does this PR introduce breaking changes?

  • Yes ⚠️
  • No

Testing:

  • Added/updated tests for my changes
  • Tested the changes manually
  • This PR is not tested ❌ (please explain why)

Code Quality:

  • Signed off every commit (git commit -s)
  • Ran pre-commit hooks (setup guide)

Documentation:

  • Updated documentation (if applicable) (contribution guide)
  • Added new APIs to doc/source/ (if applicable)

@as-jding as-jding requested a review from a team as a code owner October 15, 2025 22:01
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

This PR adds a note to the example notebooks about customizing training configurations. This is a helpful addition for users. However, the file paths provided in the notes are incorrect relative to the notebook's location. I've provided suggestions to correct these paths to match what's used in the example commands.

"\n",
"### Configure LLaMA-Factory with Ray\n",
"\n",
"**Note**: To customize the training configuration, edit `train-configs/dpo_qlora.yaml`. \n",
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 path to the configuration file seems incorrect. Later in the notebook (in the cell starting at line 214), the command used is llamafactory-cli train ../train-configs/dpo_qlora.yaml. This suggests the correct relative path from the notebook's directory is ../train-configs/dpo_qlora.yaml, not train-configs/dpo_qlora.yaml.

Suggested change
"**Note**: To customize the training configuration, edit `train-configs/dpo_qlora.yaml`. \n",
"**Note**: To customize the training configuration, edit `../train-configs/dpo_qlora.yaml`. \n",

"\n",
"### Configure LLaMA-Factory with Ray\n",
"\n",
"**Note**: To customize the training configuration, edit `train-configs/kto_lora.yaml`. \n",
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 path to the configuration file seems incorrect. The command used later in this notebook (in the cell starting at line 231) is llamafactory-cli train ../train-configs/kto_lora.yaml. This suggests the correct relative path from the notebook's directory is ../train-configs/kto_lora.yaml, not train-configs/kto_lora.yaml.

Suggested change
"**Note**: To customize the training configuration, edit `train-configs/kto_lora.yaml`. \n",
"**Note**: To customize the training configuration, edit `../train-configs/kto_lora.yaml`. \n",

"### LLaMA-Factory + Ray configuration\n",
"### Configure LLaMA-Factory with Ray\n",
"\n",
"**Note**: To customize the training configuration, edit `train-configs/sft_lora_deepspeed.yaml`. \n",
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 path to the configuration file seems incorrect. The command used later in this notebook (in the cell starting at line 308) is llamafactory-cli train ../train-configs/sft_lora_deepspeed.yaml. This suggests the correct relative path from the notebook's directory is ../train-configs/sft_lora_deepspeed.yaml, not train-configs/sft_lora_deepspeed.yaml.

Suggested change
"**Note**: To customize the training configuration, edit `train-configs/sft_lora_deepspeed.yaml`. \n",
"**Note**: To customize the training configuration, edit `../train-configs/sft_lora_deepspeed.yaml`. \n",

@kunling-anyscale kunling-anyscale added the go add ONLY when ready to merge, run all tests label Oct 15, 2025
Signed-off-by: as-jding <[email protected]>
@ray-gardener ray-gardener bot added docs An issue or change related to documentation train Ray Train Related Issue labels Oct 16, 2025
Copy link
Contributor

@angelinalg angelinalg left a comment

Choose a reason for hiding this comment

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

stamp

@angelinalg angelinalg merged commit 6ffb434 into ray-project:master Oct 16, 2025
6 checks passed
xinyuangui2 pushed a commit to xinyuangui2/ray that referenced this pull request Oct 22, 2025
elliot-barn pushed a commit that referenced this pull request Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs An issue or change related to documentation go add ONLY when ready to merge, run all tests train Ray Train Related Issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants