ronantakizawa/codereview-bench huggingface dataset:
Repository: vllm-project/vllm
File: vllm/multimodal/media/audio.py (Python)

Code before:
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
import base64
from io import BytesIO
from pathlib import Path

import av
import librosa
import numpy as np
import numpy.typing as npt
import pybase64
import soundfile
import torch

from vllm.utils.serial_utils import tensor2base64

from .base import MediaIO


def extract_audio_from_video_bytes(
    data: bytes,
) -> tuple[npt.NDArray, float]:
    """Extract the audio track from raw video bytes using PyAV.

    PyAV wraps FFmpeg's C libraries in-process — no subprocess is
    spawned, which is critical to avoid crashing CUDA-active vLLM
    worker processes.

    The returned waveform is at the native sample rate of the video's
    audio stream.  Resampling to a model-specific rate is left to the
    downstream :class:`AudioResampler` in the parsing pipeline.

    Args:
        data: Raw video file bytes (e.g. from an mp4 file).

    Returns:
        A tuple of ``(waveform, sample_rate)`` suitable for use as an

Review comment (suggestion):
Please revert the changes to the imports. Additional, `av` should use `PlaceholderModule` as it is an optional dependency.

Code after:
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
import base64
from io import BytesIO
from pathlib import Path

import numpy as np
import numpy.typing as npt
import pybase64
import torch

from vllm.utils.import_utils import PlaceholderModule
from vllm.utils.serial_utils import tensor2base64

from .base import MediaIO

try:
    import librosa
except ImportError:
    librosa = PlaceholderModule("librosa")  # type: ignore[assignment]

try:
    import soundfile
except ImportError:
    soundfile = PlaceholderModule("soundfile")  # type: ignore[assignment]

try:
    import av
except ImportError:
    av = PlaceholderModule("av")  # type: ignore[assignment]


def extract_audio_from_video_bytes(
    data: bytes,
) -> tuple[npt.NDArray, float]:
    """Extract the audio track from raw video bytes using PyAV.
Repository: vllm-project/vllm
File: tests/models/language/generation_ppl_test/test_qwen.py (Python)

Code before:
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project

import pytest

from tests.models.utils import GenerateModelInfo

from .ppl_utils import wikitext_ppl_test

MODELS = [
    # for Qwen3
    GenerateModelInfo("Qwen/Qwen3-0.6B", hf_ppl=23.864173889160156),
    GenerateModelInfo("Qwen/Qwen3-0.6B-FP8", hf_ppl=24.313045501708984),
    # for Qwen3.5
    GenerateModelInfo("Qwen/Qwen3.5-0.8B", hf_ppl=19.38858413696289),
]


@pytest.mark.parametrize("model_info", MODELS)
def test_ppl(hf_runner, vllm_runner, model_info: GenerateModelInfo):
    wikitext_ppl_test(hf_runner, vllm_runner, model_info)


Review comment (suggestion):
Can you pass `language_model_only=True` to Qwen3.5's vllm runner? I think we can skip encoder initialization and mm profiling for pure text PPL test here.

Code after:
# SPDX-License-Identifier: Apache-2.0
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project

import pytest

from tests.models.utils import GenerateModelInfo

from .ppl_utils import wikitext_ppl_test

MODELS = [
    # for Qwen3
    GenerateModelInfo("Qwen/Qwen3-0.6B", hf_ppl=23.864173889160156),
    GenerateModelInfo("Qwen/Qwen3-0.6B-FP8", hf_ppl=24.313045501708984),
    # for Qwen3.5
    GenerateModelInfo("Qwen/Qwen3.5-0.8B", hf_ppl=19.38858413696289),
]


@pytest.mark.parametrize("model_info", MODELS)
def test_ppl(hf_runner, vllm_runner, model_info: GenerateModelInfo):
    vllm_extra_kwargs = {}
    if model_info.name == "Qwen/Qwen3.5-0.8B":
        vllm_extra_kwargs["language_model_only"] = True

    wikitext_ppl_test(
        hf_runner, vllm_runner, model_info, vllm_extra_kwargs=vllm_extra_kwargs
    )
Repository: volcengine/verl
File: verl/utils/device.py (Python)

Code before:
# All rights reserved.
#
# This source code is licensed under the BSD-style license in https://github.com/pytorch/torchtune/blob/main/LICENSE

import logging

import torch

logger = logging.getLogger(__name__)


def is_torch_npu_available() -> bool:
    """Check the availability of NPU"""
    try:
        import torch_npu  # noqa: F401

        return torch.npu.is_available()
    except ImportError:
        return False


is_cuda_available = torch.cuda.is_available()
is_npu_available = is_torch_npu_available()


def get_visible_devices_prefix() -> str:
    """Function that gets visible devices prefix name.
    Returns:
        'CUDA' or 'ASCEND_RT'
    """
    return "CUDA" if is_cuda_available else "ASCEND_RT"


def get_device_name() -> str:
    """Function that gets the torch.device based on the current machine.
    This currently only supports CPU, CUDA, NPU.
    Returns:
        device
    """
    if is_cuda_available:
        device = "cuda"
    elif is_npu_available:
        device = "npu"
    else:
        device = "cpu"
    return device


def get_torch_device() -> any:
    """Return the corresponding torch attribute based on the device type string.
    Returns:

Review comment (suggestion):
Shall we modify this to directly return CUDA_VISIBLE_DEVICES to avoid performing string manipulation, which is error-prone.

Code after:
# All rights reserved.
#
# This source code is licensed under the BSD-style license in https://github.com/pytorch/torchtune/blob/main/LICENSE

import logging

import torch

logger = logging.getLogger(__name__)


def is_torch_npu_available() -> bool:
    """Check the availability of NPU"""
    try:
        import torch_npu  # noqa: F401

        return torch.npu.is_available()
    except ImportError:
        return False


is_cuda_available = torch.cuda.is_available()
is_npu_available = is_torch_npu_available()


def get_visible_devices_keyword() -> str:
    """Function that gets visible devices keyword name.
    Returns:
        'CUDA_VISIBLE_DEVICES' or `ASCEND_RT_VISIBLE_DEVICES`
    """
    return "CUDA_VISIBLE_DEVICES" if is_cuda_available else "ASCEND_RT_VISIBLE_DEVICES"


def get_device_name() -> str:
    """Function that gets the torch.device based on the current machine.
    This currently only supports CPU, CUDA, NPU.
    Returns:
        device
    """
    if is_cuda_available:
        device = "cuda"
    elif is_npu_available:
        device = "npu"
    else:
        device = "cpu"
    return device


def get_torch_device() -> any:
    """Return the corresponding torch attribute based on the device type string.
    Returns:
Repository: volcengine/verl
File: tests/special_npu/run_qwen3_30b_dapo_mindspeed.sh (Shell)

Code before:
#!/usr/bin/env bash
set -xeuo pipefail

MODEL_ID=${MODEL_ID:-Qwen/Qwen3-30B-A3B-Instruct}
MODEL_PATH=${MODEL_PATH:-${HOME}/models/${MODEL_ID}}
USE_DIST_CKPT=${USE_DIST_CKPT:-False}
DIST_CKPT_PATH=${DIST_CKPT_PATH:-${HOME}/dist_ckpt/qwen3_30b_dapo_mindspeed}

if [[ "$USE_DUMMY_MODEL" == "True" ]]; then
    DUMMY_MODEL_PATH=${DUMMY_MODEL_PATH:-${HOME}/models_dummy/${MODEL_ID}}
    if [ -z "${DUMMY_MODEL_CONFIG_PATH}" ]; then
        echo "[ERROR] DUMMY_MODEL_CONFIG_PATH not set"
        exit 1
    fi
    if [[ -d $DUMMY_MODEL_PATH && $DUMMY_MODEL_PATH != "/" ]]; then
        rm -rf $DUMMY_MODEL_PATH
    fi

    python scripts/init_random_model.py \
        --hf_model_path "${MODEL_PATH}" \
        --new_config_path "${DUMMY_MODEL_CONFIG_PATH}" \
        --output_path "${DUMMY_MODEL_PATH}"
    MODEL_PATH=$DUMMY_MODEL_PATH
fi


if [[ "$USE_DIST_CKPT" == "True" ]]; then

    if [[ "$USE_DUMMY_MODEL" == "True" ]]; then

Review comment (suggestion):
model name should be `Qwen3-30B-A3B-Instruct-2507`, see https://huggingface.co/Qwen/Qwen3-30B-A3B-Instruct-2507

Code after:
#!/usr/bin/env bash
set -xeuo pipefail

export VLLM_ASCEND_ENABLE_NZ=0

MODEL_ID=${MODEL_ID:-Qwen/Qwen3-30B-A3B-Instruct-2507}
MODEL_PATH=${MODEL_PATH:-${HOME}/models/${MODEL_ID}}
USE_DIST_CKPT=${USE_DIST_CKPT:-False}
DIST_CKPT_PATH=${DIST_CKPT_PATH:-${HOME}/dist_ckpt/qwen3_30b_dapo_mindspeed}

# use dummy model
if [[ "$USE_DUMMY_MODEL" == "True" ]]; then
    DUMMY_MODEL_PATH=${DUMMY_MODEL_PATH:-${HOME}/models_dummy/${MODEL_ID}}
    if [ -z "${DUMMY_MODEL_CONFIG_PATH}" ]; then
        echo "[ERROR] DUMMY_MODEL_CONFIG_PATH not set"
        exit 1
    fi

    # make sure the path is empty
    if [[ -d $DUMMY_MODEL_PATH && $DUMMY_MODEL_PATH != "/" ]]; then
        rm -rf $DUMMY_MODEL_PATH
    fi

    # init model
    python scripts/init_random_model.py \
        --hf_model_path "${MODEL_PATH}" \
        --new_config_path "${DUMMY_MODEL_CONFIG_PATH}" \
        --output_path "${DUMMY_MODEL_PATH}"
Repository: volcengine/verl
File: tests/special_npu/run_qwen3_30b_dapo_mindspeed.sh (Shell)

Code before:
    actor_rollout_ref.ref.strategy=megatron \
    actor_rollout_ref.ref.log_prob_micro_batch_size_per_gpu=16 \
    actor_rollout_ref.ref.megatron.pipeline_model_parallel_size=2 \
    actor_rollout_ref.ref.megatron.tensor_model_parallel_size=2 \
    actor_rollout_ref.ref.megatron.expert_model_parallel_size=2 \
    actor_rollout_ref.ref.megatron.use_dist_checkpointing=${USE_DIST_CKPT} \
    actor_rollout_ref.ref.megatron.dist_checkpointing_path=${DIST_CKPT_PATH} \
    reward_model.reward_manager=dapo \
    algorithm.kl_ctrl.kl_coef=0.0 \
    trainer.logger=['console'] \
    trainer.project_name='verl_gsm8k_example' \
    trainer.experiment_name='qwen3_30b_a3b_cut_gsm8k_mindspeed' \
    trainer.n_gpus_per_node=16 \
    trainer.nnodes=1 \
    trainer.save_freq=-1 \
    trainer.test_freq=-1 \
    trainer.total_epochs=1 \
    trainer.total_training_steps=2 \
    trainer.device=npu \
    actor_rollout_ref.actor.use_torch_compile=False \
    actor_rollout_ref.ref.use_torch_compile=False \
    +actor_rollout_ref.actor.megatron.override_transformer_config.use_flash_attn=True $@

# clean up
if [[ "$USE_DIST_CKPT" == "True" ]]; then
    rm -rf $DIST_CKPT_PATH
fi
if [[ "$USE_DUMMY_MODEL" == "True" ]]; then
    rm -rf $DUMMY_MODEL_PATH
fi


Review comment (suggestion):
For user who run this shell, after finish running it, should `DIST_CKPT_PATH` and `DUMMY_MODEL_PATH` be removed actually? If not, suggest moving `rm` operation up to e2e_ascend script. If removing is required, just ignore this comment :)

Code after:
    actor_rollout_ref.ref.strategy=megatron \
    actor_rollout_ref.ref.log_prob_micro_batch_size_per_gpu=16 \
    actor_rollout_ref.ref.megatron.pipeline_model_parallel_size=2 \
    actor_rollout_ref.ref.megatron.tensor_model_parallel_size=2 \
    actor_rollout_ref.ref.megatron.expert_model_parallel_size=2 \
    actor_rollout_ref.ref.megatron.use_dist_checkpointing=${USE_DIST_CKPT} \
    actor_rollout_ref.ref.megatron.dist_checkpointing_path=${DIST_CKPT_PATH} \
    reward_model.reward_manager=dapo \
    algorithm.kl_ctrl.kl_coef=0.0 \
    trainer.logger=['console'] \
    trainer.project_name='verl_gsm8k_example' \
    trainer.experiment_name='qwen3_30b_a3b_cut_gsm8k_mindspeed' \
    trainer.n_gpus_per_node=16 \
    trainer.nnodes=1 \
    trainer.save_freq=-1 \
    trainer.test_freq=-1 \
    trainer.total_epochs=1 \
    trainer.total_training_steps=2 \
    trainer.device=npu \
    actor_rollout_ref.actor.use_torch_compile=False \
    actor_rollout_ref.ref.use_torch_compile=False \
    +actor_rollout_ref.actor.megatron.override_transformer_config.use_flash_attn=True $@

# clean up
if [[ "$USE_DUMMY_MODEL" == "True" ]]; then
    rm -rf $DUMMY_MODEL_PATH
    if [[ "$USE_DIST_CKPT" == "True" ]]; then
        rm -rf $DIST_CKPT_PATH
    fi
fi
Repository: volcengine/verl
File: verl/utils/config.py (Python)

Code before:
    if use_critic:
        critic_config = omega_conf_to_dataclass(config.critic)
        critic_config.validate(n_gpus, config.data.train_batch_size)

    if config.data.get("val_batch_size", None) is not None:
        print(
            "WARNING: val_batch_size is deprecated."
            + " Validation datasets are sent to inference engines as a whole batch,"
            + " which will schedule the memory themselves."
        )

    # check eval config
    if config.actor_rollout_ref.rollout.val_kwargs.do_sample:
        assert config.actor_rollout_ref.rollout.temperature > 0, (
            "validation gen temperature should be greater than 0 when enabling do_sample"
        )

    # check LoRA rank in vLLM
    lora_config = config.actor_rollout_ref.model.get("lora", {})
    lora_rank = lora_config.get("rank", 0)
    if lora_config.get("merge", False):
        lora_rank = 0
    if lora_rank <= 0:
        lora_rank = config.actor_rollout_ref.model.get("lora_rank", 0)
    if lora_rank > 0 and config.actor_rollout_ref.rollout.name == "vllm":
        assert lora_rank <= 512, "LoRA rank in vLLM must be less than or equal to 512"

    print("[validate_config] All configuration checks passed successfully!")


Review comment (suggestion):
is it possible to use a number from vllm, instead of a hardcode 512? such as something like `assert lora_rank < vllm.xxx.xxx.max_lora_rank`

Code after:
    if use_critic:
        critic_config = omega_conf_to_dataclass(config.critic)
        critic_config.validate(n_gpus, config.data.train_batch_size)

    if config.data.get("val_batch_size", None) is not None:
        print(
            "WARNING: val_batch_size is deprecated."
            + " Validation datasets are sent to inference engines as a whole batch,"
            + " which will schedule the memory themselves."
        )

    # check eval config
    if config.actor_rollout_ref.rollout.val_kwargs.do_sample:
        assert config.actor_rollout_ref.rollout.temperature > 0, (
            "validation gen temperature should be greater than 0 when enabling do_sample"
        )

    # check LoRA rank in vLLM
    lora_config = config.actor_rollout_ref.model.get("lora", {})
    lora_rank = lora_config.get("rank", 0)
    if lora_config.get("merge", False):
        lora_rank = 0
    if lora_rank <= 0:
        lora_rank = config.actor_rollout_ref.model.get("lora_rank", 0)
    if lora_rank > 0 and config.actor_rollout_ref.rollout.name == "vllm":
        from verl.workers.rollout.vllm_rollout.utils import get_vllm_max_lora_rank

        get_vllm_max_lora_rank(lora_rank)

    print("[validate_config] All configuration checks passed successfully!")
Repository: vuejs/vue-next
File: packages/compiler-core/src/transforms/vFor.ts (TypeScript)

Code before:
  TransformContext
} from '../transform'
import {
  NodeTypes,
  ExpressionNode,
  createExpression,
  SourceLocation
} from '../ast'
import { createCompilerError, ErrorCodes } from '../errors'
import { getInnerRange } from '../utils'
import { RENDER_LIST } from '../runtimeConstants'
import { processExpression } from './transformExpression'

export const transformFor = createStructuralDirectiveTransform(
  'for',
  (node, dir, context) => {
    if (dir.exp) {
      const parseResult = parseForExpression(dir.exp, context)

      if (parseResult) {
        context.helper(RENDER_LIST)
        const { source, value, key, index } = parseResult

        context.replaceNode({
          type: NodeTypes.FOR,
          loc: node.loc,
          source,
          valueAlias: value,
          keyAlias: key,
          objectIndexAlias: index,
          children: [node]
        })

        // scope management
        const { addIdentifier, removeIdentifier } = context

        // inject identifiers to context
        value && addIdentifier(value)
        key && addIdentifier(key)
        index && addIdentifier(index)

        return () => {
          // remove injected identifiers on exit
          value && removeIdentifier(value)
          key && removeIdentifier(key)
          index && removeIdentifier(index)
        }
      } else {
        context.onError(
          createCompilerError(ErrorCodes.X_FOR_MALFORMED_EXPRESSION, dir.loc)
        )

Review comment (suggestion):
This gives a more accurate location for `v-for` and even allows setting breakpoint separately for v-for and the element it's applied to.

Code after:
  TransformContext
} from '../transform'
import {
  NodeTypes,
  ExpressionNode,
  createExpression,
  SourceLocation
} from '../ast'
import { createCompilerError, ErrorCodes } from '../errors'
import { getInnerRange } from '../utils'
import { RENDER_LIST } from '../runtimeConstants'
import { processExpression } from './transformExpression'

export const transformFor = createStructuralDirectiveTransform(
  'for',
  (node, dir, context) => {
    if (dir.exp) {
      const parseResult = parseForExpression(dir.exp, context)

      if (parseResult) {
        context.helper(RENDER_LIST)
        const { source, value, key, index } = parseResult

        context.replaceNode({
          type: NodeTypes.FOR,
          loc: dir.loc,
          source,
          valueAlias: value,
          keyAlias: key,
          objectIndexAlias: index,
          children: [node]
        })

        // scope management
        const { addIdentifier, removeIdentifier } = context

        // inject identifiers to context
        value && addIdentifier(value)
        key && addIdentifier(key)
        index && addIdentifier(index)

        return () => {
          // remove injected identifiers on exit
          value && removeIdentifier(value)
          key && removeIdentifier(key)
          index && removeIdentifier(index)
        }
      } else {
        context.onError(
          createCompilerError(ErrorCodes.X_FOR_MALFORMED_EXPRESSION, dir.loc)
        )
Repository: vuejs/vue-next
File: packages/runtime-core/src/componentProxy.ts (TypeScript)

Code before:
                return target.update
              case '$nextTick':
                return nextTick
              case '$watch':
                return instanceWatch.bind(target)
            }
          }
          return target.user[key]
      }
    }
  },
  // this trap is only called in browser-compiled render functions that use
  // `with (this) {}`
  has(_: any, key: string): boolean {
    return key[0] !== '_' && !globalsWhitelist.has(key)
  },
  set(target: ComponentInternalInstance, key: string, value: any): boolean {
    const { data, renderContext } = target
    if (data !== EMPTY_OBJ && hasOwn(data, key)) {
      data[key] = value
    } else if (hasOwn(renderContext, key)) {
      renderContext[key] = value
    } else if (key[0] === '$' && key.slice(1) in target) {
      __DEV__ &&
        warn(
          `Attempting to mutate public property "${key}" to "${value}"`,
          target
        )
      return false
    } else if (key in target.props) {
      __DEV__ &&
        warn(`Attempting to mutate prop "${key}" to "${value}"`, target)
      return false
    } else {
      target.user[key] = value
    }
    return true
  }
}


Review comment (suggestion):
```suggestion
          `Attempting to mutate public property "${key}". ` +
          `Properties starting with $ are reserved and readonly.`,
```

Code after:
              case '$forceUpdate':
                return target.update
              case '$nextTick':
                return nextTick
              case '$watch':
                return instanceWatch.bind(target)
            }
          }
          return target.user[key]
      }
    }
  },
  // this trap is only called in browser-compiled render functions that use
  // `with (this) {}`
  has(_: any, key: string): boolean {
    return key[0] !== '_' && !globalsWhitelist.has(key)
  },
  set(target: ComponentInternalInstance, key: string, value: any): boolean {
    const { data, renderContext } = target
    if (data !== EMPTY_OBJ && hasOwn(data, key)) {
      data[key] = value
    } else if (hasOwn(renderContext, key)) {
      renderContext[key] = value
    } else if (key[0] === '$' && key.slice(1) in target) {
      __DEV__ &&
        warn(
          `Attempting to mutate public property "${key}". ` +
            `Properties starting with $ are reserved and readonly.`,
          target
        )
      return false
    } else if (key in target.props) {
      __DEV__ &&
        warn(`Attempting to mutate prop "${key}". Props are readonly.`, target)
      return false
    } else {
      target.user[key] = value
    }
    return true
  }
}
Repository: vuejs/vue-next
File: packages/runtime-core/src/componentProxy.ts (TypeScript)

Code before:
          return target.user[key]
      }
    }
  },
  // this trap is only called in browser-compiled render functions that use
  // `with (this) {}`
  has(_: any, key: string): boolean {
    return key[0] !== '_' && !globalsWhitelist.has(key)
  },
  set(target: ComponentInternalInstance, key: string, value: any): boolean {
    const { data, renderContext } = target
    if (data !== EMPTY_OBJ && hasOwn(data, key)) {
      data[key] = value
    } else if (hasOwn(renderContext, key)) {
      renderContext[key] = value
    } else if (key[0] === '$' && key.slice(1) in target) {
      __DEV__ &&
        warn(
          `Attempting to mutate public property "${key}".` +
          `Properties starting with $ are reserved and readonly.`,
          target
        )
      return false
    } else if (key in target.props) {
      __DEV__ &&
        warn(`Attempting to mutate prop "${key}" to "${value}"`, target)
      return false
    } else {
      target.user[key] = value
    }
    return true
  }
}


Review comment (suggestion):
```suggestion
        warn(
          `Attempting to mutate prop "${key}". Props are readonly.`,
          target
        )
```

Code after:
          }
          return target.user[key]
      }
    }
  },
  // this trap is only called in browser-compiled render functions that use
  // `with (this) {}`
  has(_: any, key: string): boolean {
    return key[0] !== '_' && !globalsWhitelist.has(key)
  },
  set(target: ComponentInternalInstance, key: string, value: any): boolean {
    const { data, renderContext } = target
    if (data !== EMPTY_OBJ && hasOwn(data, key)) {
      data[key] = value
    } else if (hasOwn(renderContext, key)) {
      renderContext[key] = value
    } else if (key[0] === '$' && key.slice(1) in target) {
      __DEV__ &&
        warn(
          `Attempting to mutate public property "${key}". ` +
            `Properties starting with $ are reserved and readonly.`,
          target
        )
      return false
    } else if (key in target.props) {
      __DEV__ &&
        warn(`Attempting to mutate prop "${key}". Props are readonly.`, target)
      return false
    } else {
      target.user[key] = value
    }
    return true
  }
}