Skip to content

Conversation

@chenyushuo
Copy link
Collaborator

@chenyushuo chenyushuo commented Nov 19, 2025

Description

  1. Bug fix when loading checkpoint in bench mode.
  2. Bug fix NotImplementedError when saving huggingface model by Megatron engine.
  3. Fix kl_loss != 0 in first step.
  4. Remove kl_loss_coef and kl_loss_type in verl_config.
  5. Check micro_batch_size when not using use_dynamic_bsz in fsdp_workers.
  6. Fix mode check.
  7. Fix bots_reward.py

Checklist

Please check the following items before code is ready to be reviewed.

  • Code has passed all tests
  • Docstrings have been added/updated in Google Style
  • Documentation has been updated
  • Code is ready for review

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @chenyushuo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses two critical bug fixes related to model management. It refines the checkpoint loading mechanism to prevent erroneous behavior in benchmark mode and enhances the stability of Hugging Face model saving by introducing comprehensive error handling. These changes contribute to more reliable and robust model operations within the system.

Highlights

  • Benchmark Checkpoint Loading: Fixed an issue where the _find_latest_state_dict task was incorrectly initiated in benchmark mode, preventing proper checkpoint loading. The task will now only run when not in 'bench' mode.
  • Hugging Face Model Saving: Implemented robust error handling for Hugging Face model saving within the Megatron engine. The saving process is now wrapped in a try-except block, catching potential exceptions (like NotImplementedError) and logging an informative error message that suggests using use_mbridge=true as an alternative.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

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 pull request introduces two bug fixes. The first prevents the synchronizer from automatically loading the latest checkpoint when running in 'bench' mode, which is a correct change to allow benchmarking specific checkpoints. The second change adds error handling when saving Hugging Face models with the Megatron engine, preventing crashes from NotImplementedError and providing a helpful message to the user. I've suggested a small improvement to the error logging to include a full traceback for easier debugging. Overall, the changes improve the robustness and correctness of the codebase.

@chenyushuo
Copy link
Collaborator Author

/unittest-all

@chenyushuo
Copy link
Collaborator Author

/unittest-all

@github-actions
Copy link

Summary

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Other ❓ Flaky 🍂 Duration ⏱️
178 175 0 3 0 0 1h 17m

Skipped

Tests Status
tests/explorer/workflow_test.py::TestAgentScopeWorkflowAdapter::test_adapter skipped ⏭️
tests/trainer/trainer_test.py::TestMultiModalGRPO::test_trainer skipped ⏭️
tests/trainer/trainer_test.py::TestMultiModalSFT::test_trainer skipped ⏭️

Tests

Test Name Status Flaky Duration
tests/algorithm/advantage_fn_test.py::TestGroupedAdvantageFn::test_batch_level_std_grpo 40ms
tests/algorithm/advantage_fn_test.py::TestGroupedAdvantageFn::test_batch_level_step_wise_grpo_advantage 2ms
tests/algorithm/advantage_fn_test.py::TestGroupedAdvantageFn::test_duplicate_grpo 5ms
tests/algorithm/advantage_fn_test.py::TestGroupedAdvantageFn::test_grpo_advantage 3ms
tests/algorithm/advantage_fn_test.py::TestGroupedAdvantageFn::test_grpo_correct_bias 2ms
tests/algorithm/advantage_fn_test.py::TestGroupedAdvantageFn::test_grpo_reward_std 1ms
tests/algorithm/advantage_fn_test.py::TestGroupedAdvantageFn::test_step_wise_grpo_advantage 2ms
tests/algorithm/advantage_fn_test.py::TestGroupedAdvantageFn::test_step_wise_grpo_with_std_threshold 2ms
tests/algorithm/policy_loss_test.py::VerlPolicyLossTest::test_dpo_policy_loss 2ms
tests/algorithm/policy_loss_test.py::VerlPolicyLossTest::test_gspo_policy_loss 1ms
tests/algorithm/policy_loss_test.py::VerlPolicyLossTest::test_mix_policy_loss 1ms
tests/algorithm/policy_loss_test.py::VerlPolicyLossTest::test_opmd_policy_loss 1ms
tests/algorithm/policy_loss_test.py::VerlPolicyLossTest::test_ppo_policy_loss 1ms
tests/algorithm/policy_loss_test.py::VerlPolicyLossTest::test_sft_policy_loss 1ms
tests/buffer/experience_pipeline_test.py::TestExperiencePipeline::test_experience_pipeline 19.6s
tests/buffer/experience_pipeline_test.py::TestExperiencePipeline::test_pass_rate_calculation 15.5s
tests/buffer/experience_storage_test.py::ExperienceStorageTest::test_sql_experience_buffer 4.1s
tests/buffer/experience_storage_test.py::ExperienceStorageTest::test_sql_storage_0_sft 5.9s
tests/buffer/experience_storage_test.py::ExperienceStorageTest::test_sql_storage_1_dpo 6.7s
tests/buffer/file_test.py::TestFileBuffer::test_file_reader 154ms
tests/buffer/file_test.py::TestFileBuffer::test_file_writer 4.2s
tests/buffer/formatter_test.py::TestFormatter::test_dpo_messages_formatter 509ms
tests/buffer/formatter_test.py::TestFormatter::test_dpo_plaintext_formatter 452ms
tests/buffer/formatter_test.py::TestFormatter::test_multi_modal_sft_formatter 767ms
tests/buffer/formatter_test.py::TestFormatter::test_sft_messages_formatter 956ms
tests/buffer/formatter_test.py::TestFormatter::test_sft_plaintext_formatter 704ms
tests/buffer/formatter_test.py::TestFormatter::test_task_formatter 238ms
tests/buffer/queue_test.py::TestQueueBuffer::test_priority_queue_buffer_reuse 8.7s
tests/buffer/queue_test.py::TestQueueBuffer::test_priority_queue_capacity 5.0s
tests/buffer/queue_test.py::TestQueueBuffer::test_priority_queue_reuse_count_control 6.8s
tests/buffer/queue_test.py::TestQueueBuffer::test_queue_buffer_0_queue 5.8s
tests/buffer/queue_test.py::TestQueueBuffer::test_queue_buffer_1_priority_queue 5.7s
tests/buffer/queue_test.py::TestQueueBuffer::test_queue_buffer_capacity 6.4s
tests/buffer/reward_shaping_mapper_test.py::TestRewardShapingMapper::test_basic_usage 5ms
tests/buffer/sql_test.py::TestSQLBuffer::test_sql_buffer_read_write 4.7s
tests/buffer/task_scheduler_test.py::TestTaskScheduler::test_task_scheduler_0 89ms
tests/buffer/task_scheduler_test.py::TestTaskScheduler::test_task_scheduler_1 68ms
tests/buffer/task_scheduler_test.py::TestTaskScheduler::test_task_scheduler_2 106ms
tests/buffer/task_scheduler_test.py::TestTaskScheduler::test_task_scheduler_3 107ms
tests/buffer/task_scheduler_test.py::TestTaskScheduler::test_task_scheduler_4 107ms
tests/buffer/task_scheduler_test.py::TestTaskScheduler::test_task_scheduler_5 112ms
tests/buffer/task_scheduler_test.py::TestTaskScheduler::test_task_scheduler_6 128ms
tests/buffer/task_storage_test.py::TaskStorageTest::test_read_task_0 69ms
tests/buffer/task_storage_test.py::TaskStorageTest::test_read_task_1 3.9s
tests/buffer/task_storage_test.py::TaskStorageTest::test_read_task_2 49ms
tests/buffer/task_storage_test.py::TaskStorageTest::test_read_task_3 3.9s
tests/buffer/task_storage_test.py::TaskStorageTest::test_read_task_4 49ms
tests/buffer/task_storage_test.py::TaskStorageTest::test_read_task_5 4.6s
tests/cli/launcher_test.py::TestLauncherMain::test_debug_mode 46.1s
tests/cli/launcher_test.py::TestLauncherMain::test_main_run_command 6.7s
tests/cli/launcher_test.py::TestLauncherMain::test_main_run_in_dlc 1.4s
tests/cli/launcher_test.py::TestLauncherMain::test_main_studio_command 332ms
tests/cli/launcher_test.py::TestLauncherMain::test_multi_stage_run 1.7s
tests/common/config_test.py::TestConfig::test_all_examples_are_valid 33.4s
tests/common/config_test.py::TestConfig::test_chat_template_path 92ms
tests/common/config_test.py::TestConfig::test_config_flatten 39ms
tests/common/config_test.py::TestConfig::test_continue_from_checkpoint_is_valid 191ms
tests/common/config_test.py::TestConfig::test_default_workflow 92ms
tests/common/config_test.py::TestConfig::test_load_default_config 23.7s
tests/common/config_test.py::TestConfig::test_max_token_len_per_gpu_set_correctly 92ms
tests/common/config_test.py::TestConfig::test_optimizer_config_propagation 91ms
tests/common/config_test.py::TestConfig::test_update_config_from_ray_cluster 355ms
tests/common/experience_test.py::TestEID::test_eid_properties 1ms
tests/common/experience_test.py::TestExperience::test_action_mask_and_logprobs_type 1ms
tests/common/experience_test.py::TestExperience::test_assertions 1ms
tests/common/experience_test.py::TestExperience::test_dpo_experience 1ms
tests/common/experience_test.py::TestExperience::test_gather 1ms
tests/common/experience_test.py::TestExperience::test_hf_datasets_conversion 15ms
tests/common/experience_test.py::TestExperience::test_multi_turn_experience 1ms
tests/common/experience_test.py::TestExperience::test_serialize_deserialize 1ms
tests/common/experience_test.py::TestExperience::test_single_turn_experience 1ms
tests/common/experience_test.py::TestExperience::test_to_dict 1ms
tests/common/experience_test.py::TestExperienceConversion::test_batch_conversion 1ms
tests/common/experience_test.py::TestExperienceConversion::test_dpo_experience_batch_conversion 1ms
tests/common/experience_test.py::TestExperienceConversion::test_experience_model_experience_conversion 1ms
tests/common/experience_test.py::TestExperienceConversion::test_gather_experiences_with_custom_fields 1ms
tests/common/experience_test.py::TestExperienceConversion::test_multiturn_experience_batch_converstion 1ms
tests/common/vllm_test.py::ModelWrapperTest_0::test_generate 45.4s
tests/common/vllm_test.py::ModelWrapperTest_1::test_generate 31.4s
tests/common/vllm_test.py::ModelWrapperTest_2::test_generate 43.9s
tests/common/vllm_test.py::TestModelLen_0::test_model_len 17.0s
tests/common/vllm_test.py::TestModelLen_1::test_model_len 16.8s
tests/common/vllm_test.py::TestAPIServer::test_api 22.9s
tests/common/vllm_test.py::TestLogprobs::test_logprobs 19.3s
tests/common/vllm_test.py::TestAsyncAPIServer::test_api_async 23.5s
tests/common/vllm_test.py::TestTokenizer::test_action_mask 242ms
tests/common/vllm_test.py::TestTokenizer::test_action_mask_with_tools 234ms
tests/common/vllm_test.py::TestAPIServerToolCall_0_deepseek_r1::test_api_tool_calls 19.2s
tests/common/vllm_test.py::TestAPIServerToolCall_1::test_api_tool_calls 17.8s
tests/common/vllm_test.py::TestSuperLongGeneration::test_generate 3m 8s
tests/explorer/explorer_test.py::TestExplorerCountdownEval::test_explorer 1m 9s
tests/explorer/explorer_test.py::TestExplorerGSM8KRULERNoEval::test_explorer 1m 37s
tests/explorer/explorer_test.py::TestExplorerGSM8k::test_explorer 3m 39s
tests/explorer/explorer_test.py::ServeTest::test_serve 1m 21s
tests/explorer/scheduler_test.py::SchedulerTest::test_async_workflow 12.6s
tests/explorer/scheduler_test.py::SchedulerTest::test_concurrent_operations 12.3s
tests/explorer/scheduler_test.py::SchedulerTest::test_get_results 31.3s
tests/explorer/scheduler_test.py::SchedulerTest::test_multi_step_execution 12.8s
tests/explorer/scheduler_test.py::SchedulerTest::test_non_repeatable_workflow 12.1s
tests/explorer/scheduler_test.py::SchedulerTest::test_scheduler_all_methods 22.0s
tests/explorer/scheduler_test.py::SchedulerTest::test_scheduler_restart_after_stop 23.6s
tests/explorer/scheduler_test.py::SchedulerTest::test_split_tasks 15.7s
tests/explorer/scheduler_test.py::SchedulerTest::test_stepwise_experience_eid 12.7s
tests/explorer/scheduler_test.py::SchedulerTest::test_wait_all 15.5s
tests/explorer/scheduler_test.py::SchedulerTest::test_wait_all_timeout_with_multi_batch 21.4s
tests/explorer/step_wise_workflow_test.py::WorkflowTest::test_reward_propagation_workflow_0 3ms
tests/explorer/step_wise_workflow_test.py::WorkflowTest::test_reward_propagation_workflow_1 602ms
tests/explorer/step_wise_workflow_test.py::WorkflowTest::test_step_wise_reward_workflow_0 1ms
tests/explorer/step_wise_workflow_test.py::WorkflowTest::test_step_wise_reward_workflow_1 1.0s
tests/explorer/step_wise_workflow_test.py::WorkflowTest::test_workflows_raise_error 1ms
tests/explorer/step_wise_workflow_test.py::WorkflowTest::test_workflows_stop_at_max_env_steps 1.0s
tests/explorer/workflow_test.py::WorkflowTest::test_gsm8k_workflow 15ms
tests/explorer/workflow_test.py::WorkflowTest::test_math_boxed_workflow 24ms
tests/explorer/workflow_test.py::WorkflowTest::test_math_complex_workflow 267ms
tests/explorer/workflow_test.py::WorkflowTest::test_math_eval_workflow 4ms
tests/explorer/workflow_test.py::WorkflowTest::test_math_fraction_workflow 17ms
tests/explorer/workflow_test.py::WorkflowTest::test_math_workflow 10ms
tests/explorer/workflow_test.py::WorkflowTest::test_rm_gallery_workflow 81ms
tests/explorer/workflow_test.py::WorkflowTest::test_workflow_repeatable_0 1ms
tests/explorer/workflow_test.py::WorkflowTest::test_workflow_repeatable_1 101ms
tests/explorer/workflow_test.py::WorkflowTest::test_workflow_resettable_0 1ms
tests/explorer/workflow_test.py::WorkflowTest::test_workflow_resettable_1 201ms
tests/explorer/workflow_test.py::MultiTurnWorkflowTest_0::test_multi_turn_workflow 14.5s
tests/explorer/workflow_test.py::MultiTurnWorkflowTest_1::test_multi_turn_workflow 14.4s
tests/explorer/workflow_test.py::TestAgentScopeWorkflowAdapter::test_adapter ⏭️ 1ms
tests/explorer/workflow_test.py::TestWorkflowRunner::test_workflow_runner 300ms
tests/manager/synchronizer_test.py::TestSynchronizerExit::test_synchronizer 59.6s
tests/manager/synchronizer_test.py::TestStateDictBasedSynchronizer_0::test_synchronizer 1m 42s
tests/manager/synchronizer_test.py::TestStateDictBasedSynchronizer_1::test_synchronizer 1m 49s
tests/manager/synchronizer_test.py::TestStateDictBasedSynchronizer_2::test_synchronizer 2m 25s
tests/manager/synchronizer_test.py::TestStateDictBasedSynchronizer_3::test_synchronizer 2m 24s
tests/manager/synchronizer_test.py::TestNCCLBasedSynchronizer_0::test_synchronizer 1m 44s
tests/manager/synchronizer_test.py::TestNCCLBasedSynchronizer_1::test_synchronizer 1m 43s
tests/service/data_juicer_test.py::TestDataJuicer::test_config 1.3s
tests/service/data_juicer_test.py::TestDataJuicer::test_server_start 21.6s
tests/service/data_juicer_test.py::TestDataJuicerExperiencePipeline::test_data_juicer_operators 30.6s
tests/service/data_juicer_test.py::TestDataJuicerTaskPipeline::test_data_juicer_task_pipeline 14.4s
tests/trainer/trainer_test.py::TestTrainerCountdown_0_fsdp::test_trainer 2m 38s
tests/trainer/trainer_test.py::TestTrainerCountdown_1_megatron::test_trainer 4m 48s
tests/trainer/trainer_test.py::TestStepAheadAsyncRL::test_trainer 1m 24s
tests/trainer/trainer_test.py::TestTrainerGSM8K_0_fsdp::test_trainer 1m 20s
tests/trainer/trainer_test.py::TestTrainerGSM8K_1_fsdp2::test_trainer 1m 21s
tests/trainer/trainer_test.py::TestTrainerGSM8K_2_fsdp::test_trainer 1m 22s
tests/trainer/trainer_test.py::TestTrainerGSM8K_3_fsdp2::test_trainer 1m 34s
tests/trainer/trainer_test.py::TestTrainerSFTWarmupGSM8K::test_trainer 2m 27s
tests/trainer/trainer_test.py::TestTrainerDPO::test_trainer 1m 2s
tests/trainer/trainer_test.py::TestTrainerSFT::test_trainer 58.1s
tests/trainer/trainer_test.py::TestTrainerToolsSFT::test_trainer_tools 57.6s
tests/trainer/trainer_test.py::TestFullyAsyncMode_0_fsdp::test_fully_async_mode 1m 56s
tests/trainer/trainer_test.py::TestFullyAsyncMode_1_fsdp::test_fully_async_mode 1m 54s
tests/trainer/trainer_test.py::TestFullyAsyncMode_2_megatron::test_fully_async_mode 2m 56s
tests/trainer/trainer_test.py::TestTrainerCheckpointSave_0_fsdp::test_trainer 2m 19s
tests/trainer/trainer_test.py::TestTrainerCheckpointSave_1_megatron::test_trainer 5m 31s
tests/trainer/trainer_test.py::TestTrainerMIX::test_trainer 1m 22s
tests/trainer/trainer_test.py::TestMultiModalGRPO::test_trainer ⏭️ 809ms
tests/trainer/trainer_test.py::TestMultiModalSFT::test_trainer ⏭️ 808ms
tests/trainer/trainer_test.py::TestTrainerLoRA::test_trainer 3m 11s
tests/utils/eval_utils_test.py::TestComputeScore::test_both_boxed_and_equivalent 15ms
tests/utils/eval_utils_test.py::TestComputeScore::test_both_boxed_and_not_equivalent 1ms
tests/utils/eval_utils_test.py::TestComputeScore::test_empty_ground_truth 2ms
tests/utils/eval_utils_test.py::TestComputeScore::test_empty_solution_string 1ms
tests/utils/eval_utils_test.py::TestComputeScore::test_multiple_boxed_answers_in_solution 2ms
tests/utils/eval_utils_test.py::TestComputeScore::test_solution_boxed_truth_raw_and_equivalent 1ms
tests/utils/eval_utils_test.py::TestComputeScore::test_solution_boxed_truth_raw_and_not_equivalent 1ms
tests/utils/eval_utils_test.py::TestComputeScore::test_solution_not_boxed 1ms
tests/utils/eval_utils_test.py::TestComputeScore::test_solution_raw_and_ground_truth_boxed_equivalent 1ms
tests/utils/eval_utils_test.py::TestMathEvalUtils::test_extract_answer 4ms
tests/utils/eval_utils_test.py::TestMathEvalUtils::test_verify_math_answer 74ms
tests/utils/eval_utils_test.py::TestEvalUtils::test_is_equiv 6ms
tests/utils/log_test.py::LogTest::test_actor_log 5.0s
tests/utils/log_test.py::LogTest::test_group_by_node 4.8s
tests/utils/log_test.py::LogTest::test_no_actor_log 900ms
tests/utils/plugin_test.py::TestPluginLoader::test_load_plugins_local_0__workspace_tests_utils_plugins 647ms
tests/utils/plugin_test.py::TestPluginLoader::test_load_plugins_local_1_tests_utils_plugins 92ms
tests/utils/plugin_test.py::TestPluginLoader::test_load_plugins_remote_0__workspace_tests_utils_plugins 21.5s
tests/utils/plugin_test.py::TestPluginLoader::test_load_plugins_remote_1_tests_utils_plugins 21.9s
tests/utils/plugin_test.py::TestPluginLoader::test_passing_custom_class_0__workspace_tests_utils_plugins 12.0s
tests/utils/plugin_test.py::TestPluginLoader::test_passing_custom_class_1_tests_utils_plugins 11.7s

Github Test Reporter by CTRF 💚

@pan-x-c pan-x-c merged commit a690509 into modelscope:main Nov 20, 2025
1 check passed
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.

2 participants