ronantakizawa/codereview-bench huggingface dataset:
Repository: huggingface/diffusers
File: src/diffusers/guiders/magnitude_aware_guidance.py (Python)

Code before:
        if not self._enabled:
            return False

        is_within_range = True
        if self._num_inference_steps is not None:
            skip_start_step = int(self._start * self._num_inference_steps)
            skip_stop_step = int(self._stop * self._num_inference_steps)
            is_within_range = skip_start_step <= self._step < skip_stop_step

        is_close = False
        if self.use_original_formulation:
            is_close = math.isclose(self.guidance_scale, 0.0)
        else:
            is_close = math.isclose(self.guidance_scale, 1.0)

        return is_within_range and not is_close


def mambo_guidance(
    pred_cond: torch.Tensor,
    pred_uncond: torch.Tensor,
    guidance_scale: float,
    alpha: float = 8.0,
    use_original_formulation: bool = False,
):
    dim = [i for i in range(1, len(pred_cond.shape))]
    diff = pred_cond - pred_uncond
    ratio = torch.norm(diff, dim=dim, keepdim=True) / torch.norm(pred_uncond, dim=dim, keepdim=True)
    guidance_scale_final = guidance_scale * torch.exp(- alpha * ratio) if use_original_formulation else 1.0 + (guidance_scale - 1.0) * torch.exp(- alpha * ratio)
    pred = pred_cond if use_original_formulation else pred_uncond
    pred = pred + guidance_scale_final * diff

    return pred


Review comment (suggestion):
```suggestion
    dim = list(range(1, len(pred_cond.shape)))
```

Code after:
    def _is_mambo_g_enabled(self) -> bool:
        if not self._enabled:
            return False

        is_within_range = True
        if self._num_inference_steps is not None:
            skip_start_step = int(self._start * self._num_inference_steps)
            skip_stop_step = int(self._stop * self._num_inference_steps)
            is_within_range = skip_start_step <= self._step < skip_stop_step

        is_close = False
        if self.use_original_formulation:
            is_close = math.isclose(self.guidance_scale, 0.0)
        else:
            is_close = math.isclose(self.guidance_scale, 1.0)

        return is_within_range and not is_close


def mambo_guidance(
    pred_cond: torch.Tensor,
    pred_uncond: torch.Tensor,
    guidance_scale: float,
    alpha: float = 8.0,
    use_original_formulation: bool = False,
):
    dim = list(range(1, len(pred_cond.shape)))
    diff = pred_cond - pred_uncond
    ratio = torch.norm(diff, dim=dim, keepdim=True) / torch.norm(pred_uncond, dim=dim, keepdim=True)
    guidance_scale_final = (
        guidance_scale * torch.exp(-alpha * ratio)
        if use_original_formulation
        else 1.0 + (guidance_scale - 1.0) * torch.exp(-alpha * ratio)
    )
    pred = pred_cond if use_original_formulation else pred_uncond
    pred = pred + guidance_scale_final * diff

    return pred
Repository: huggingface/transformers
File: src/transformers/models/siglip/processing_siglip.py (Python)

Code before:
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""
Image/Text processor class for SigLIP.
"""

from ...processing_utils import ProcessorMixin


def _is_siglip2_checkpoint(processor) -> bool:
    """
    SigLIP2 checkpoints currently ship a SiglipConfig (model_type='siglip') and thus load SiglipProcessor.
    We detect SigLIP2 primarily via the image processor type/module, with a tokenizer
    name/path fallback.
    """
    # SigLIP2 uses Siglip2ImageProcessor / Siglip2ImageProcessorFast
    image_processor = getattr(processor, "image_processor", None)
    if image_processor is not None:
        mod = getattr(image_processor, "__module__", "") or ""
        cls_name = image_processor.__class__.__name__.lower()
        if "siglip2" in mod.lower() or cls_name.startswith("siglip2"):
            return True

    # Fallback: look at tokenizer metadata
    tokenizer = getattr(processor, "tokenizer", None)
    if tokenizer is not None:
        name_or_path = getattr(tokenizer, "name_or_path", None) or getattr(
            tokenizer, "pretrained_model_name_or_path", None
        )
        if isinstance(name_or_path, str) and "siglip2" in name_or_path:
            return True

        init_kwargs = getattr(tokenizer, "init_kwargs", {}) or {}
        maybe = init_kwargs.get("name_or_path") or init_kwargs.get("pretrained_model_name_or_path")
        if isinstance(maybe, str) and "siglip2" in maybe:
            return True

    return False


def _lowercase_text(x):
    if x is None:
        return None
    if isinstance(x, str):

Review comment (suggestion):
I think we need a Siglip2Tokenizer class in that case because it's not guaranteed that users load a whole processor when they only want to encode text samples

Code after:
# Copyright 2024 The HuggingFace Inc. team.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""
Image/Text processor class for SigLIP.
"""

from ...processing_utils import ProcessorMixin
from ...utils import auto_docstring


@auto_docstring
class SiglipProcessor(ProcessorMixin):
    def __init__(self, image_processor, tokenizer):
        super().__init__(image_processor, tokenizer)


__all__ = ["SiglipProcessor"]
Repository: huggingface/transformers
File: src/transformers/models/siglip2/tokenization_siglip2.py (Python)

Code before:
    if isinstance(x, str):
        return x.lower()
    if isinstance(x, (list, tuple)):
        return [_lowercase_text(i) for i in x]
    return x


class Siglip2Tokenizer(GemmaTokenizer):
    """
    SigLIP2 text encoder was trained on:
      - lowercased text
      - fixed padding/truncation to 64 tokens

    This tokenizer wrapper keeps Gemma's underlying vocab/tokenization,
    while applying SigLIP2-specific preprocessing defaults.
    """

    def __call__(self, text=None, text_pair=None, **kwargs):
        # SigLIP2 training: lowercase text
        text = _lowercase_text(text)
        text_pair = _lowercase_text(text_pair)

        # SigLIP2 training: fixed padding/truncation to 64
        kwargs.setdefault("padding", "max_length")
        kwargs.setdefault("truncation", True)
        kwargs.setdefault("max_length", 64)

        return super().__call__(text=text, text_pair=text_pair, **kwargs)


__all__ = ["Siglip2Tokenizer"]


Review comment (suggestion):
not sure about this. It also has to be in saved config and we can also add default values in `ModelProcessorKwargs`

Code after:


class Siglip2Tokenizer(TokenizersBackend):
    """
    Gemma tokenizer + SigLIP2 training default: lowercase normalization.
    """

    vocab_files_names = VOCAB_FILES_NAMES
    padding_side = "left"
    model_input_names = ["input_ids", "attention_mask"]
    model = BPE

    def __init__(
        self,
        vocab: str | dict[str, int] | None = None,
        merges: str | list[str] | None = None,
        unk_token: str = "<unk>",
        bos_token: str = "<bos>",
        eos_token: str = "<eos>",
        pad_token: str = "<pad>",
        mask_token: str = "<mask>",
        **kwargs,
    ):
        if vocab is None:
            vocab = {
                str(pad_token): 0,
                str(eos_token): 1,
                str(bos_token): 2,
                str(unk_token): 3,
                str(mask_token): 4,
            }
        self._vocab = vocab
        self._merges = merges or []

        self._tokenizer = Tokenizer(
            BPE(
                vocab=self._vocab,
                merges=self._merges,
                fuse_unk=True,
                unk_token=str(unk_token),
                dropout=None,
                byte_fallback=True,
            )
        )

        self._tokenizer.decoder = decoders.Sequence(
            [decoders.Replace("▁", " "), decoders.ByteFallback(), decoders.Fuse()]
        )
        self._tokenizer.normalizer = normalizers.Replace(" ", "▁")
        super().__init__(
            unk_token=unk_token,
Repository: huggingface/transformers
File: benchmark_v2/framework/hardware_metrics.py (Python)

Code before:
import logging
import subprocess
import sys
import time
from dataclasses import dataclass
from enum import Enum
from logging import Logger
from multiprocessing import Pipe, Process
from multiprocessing.connection import Connection

from transformers.utils.import_utils import is_cuda_platform, is_rocm_platform


if is_cuda_platform():
    import gpustat

if is_rocm_platform():
    import amdsmi

import psutil
import torch

from transformers.utils import is_torch_accelerator_available


# Data class to hold the hardware information
def get_device_name_and_memory_total() -> tuple[str, float]:
    """Returns the name and memory total of GPU 0."""
    device_type = torch.accelerator.current_accelerator().type if is_torch_accelerator_available() else "cuda"
    torch_accelerator_module = getattr(torch, device_type, torch.cuda)
    device_name = torch_accelerator_module.get_device_properties(0).name
    device_memory_total = torch_accelerator_module.get_device_properties(0).total_memory / 1024**3
    return device_name, device_memory_total


class HardwareInfo:
    """A class to hold information about the hardware."""

    def __init__(self) -> None:
        # Retrieve GPU stats

Review comment (suggestion):
it might also make sense to directly use pynvml since gpustat is cli wrapper around https://pypi.org/project/nvidia-ml-py but none of its cli functionalities is used here, just fyi

Code after:
import logging
import subprocess
import sys
import time
from dataclasses import dataclass
from enum import Enum
from logging import Logger
from multiprocessing import Pipe, Process
from multiprocessing.connection import Connection

from transformers.utils.import_utils import is_cuda_platform, is_rocm_platform


if is_cuda_platform():
    import pynvml

if is_rocm_platform():
    import amdsmi

import psutil
import torch

from transformers.utils import is_torch_accelerator_available


# Data class to hold the hardware information
def get_device_name_and_memory_total() -> tuple[str, float]:
    """Returns the name and memory total of GPU 0."""
    device_type = torch.accelerator.current_accelerator().type if is_torch_accelerator_available() else "cuda"
    torch_accelerator_module = getattr(torch, device_type, torch.cuda)
    device_name = torch_accelerator_module.get_device_properties(0).name
    device_memory_total = torch_accelerator_module.get_device_properties(0).total_memory / 1024**3
    return device_name, device_memory_total


class HardwareInfo:
    """A class to hold information about the hardware."""

    def __init__(self) -> None:
        # Retrieve GPU stats
Repository: huggingface/transformers
File: tests/test_tokenization_common.py (Python)

Code before:

    def test_alignment_methods(self):
        self.skipTest("SentencePiece fast tokenizers do not expose token alignment metadata.")

    def test_local_files_only(self):
        from transformers import AutoTokenizer

        pretrained_list = getattr(self, "from_pretrained_id", []) or []
        for pretrained_name in pretrained_list:
            with self.subTest(f"AutoTokenizer ({pretrained_name})"):
                # First cache the tokenizer files
                try:
                    tokenizer_cached = AutoTokenizer.from_pretrained(pretrained_name)

                    # Now load with local_files_only=True
                    tokenizer_local = AutoTokenizer.from_pretrained(pretrained_name, local_files_only=True)

                    # Check that the two tokenizers are identical
                    self.assertEqual(tokenizer_cached.get_vocab(), tokenizer_local.get_vocab())
                    self.assertEqual(
                        tokenizer_cached.all_special_tokens_extended,
                        tokenizer_local.all_special_tokens_extended,
                    )
                except Exception as e:
                    # if the pretrained model is not loadable how could it pass locally :)
                    print(f"Could not load pretrained model: {e}")


Review comment (suggestion):
```suggestion
                    print(f"Could not load pretrained tokenizer: {e}")
```

Code after:

    def test_alignment_methods(self):
        self.skipTest("SentencePiece fast tokenizers do not expose token alignment metadata.")

    def test_local_files_only(self):
        from transformers import AutoTokenizer

        pretrained_list = getattr(self, "from_pretrained_id", []) or []
        for pretrained_name in pretrained_list:
            with self.subTest(f"AutoTokenizer ({pretrained_name})"):
                # First cache the tokenizer files
                try:
                    tokenizer_cached = AutoTokenizer.from_pretrained(pretrained_name)

                    # Now load with local_files_only=True
                    tokenizer_local = AutoTokenizer.from_pretrained(pretrained_name, local_files_only=True)

                    # Check that the two tokenizers are identical
                    self.assertEqual(tokenizer_cached.get_vocab(), tokenizer_local.get_vocab())
                    self.assertEqual(
                        tokenizer_cached.all_special_tokens_extended,
                        tokenizer_local.all_special_tokens_extended,
                    )
                except Exception as e:
                    # if the pretrained model is not loadable how could it pass locally :)
                    print(f"Could not load pretrained tokenizer: {e}")
Repository: hyprwm/Hyprland
File: src/config/ConfigValue.hpp (C++)

Code before:
#pragma once

#include <string>
#include <typeindex>
#include <hyprlang.hpp>
#include "../debug/Log.hpp"
#include "../macros.hpp"
#include "ConfigManager.hpp"

template <typename T>
class CConfigValue {
  public:
    CConfigValue(const std::string& val) {
        std::lock_guard<std::mutex> guard(m_configMutex);
        const auto                  PVHYPRLANG = g_pConfigManager->getHyprlangConfigValuePtr(val);

        p_ = PVHYPRLANG->getDataStaticPtr();

#ifdef HYPRLAND_DEBUG
        // verify type
        const auto ANY  = PVHYPRLANG->getValue();
        const auto TYPE = std::type_index(ANY.type());

        // exceptions
        const bool STRINGEX = (typeid(T) == typeid(std::string) && TYPE == typeid(Hyprlang::STRING));
        const bool CUSTOMEX = (typeid(T) == typeid(Hyprlang::CUSTOMTYPE) && (TYPE == typeid(Hyprlang::CUSTOMTYPE*) || TYPE == typeid(void*) /* dunno why it does this? */));

        RASSERT(typeid(T) == TYPE || STRINGEX || CUSTOMEX, "Mismatched type in CConfigValue<T>, got {} but has {}", typeid(T).name(), TYPE.name());
#endif
    }

    T* ptr() const {
        return *(T* const*)p_;
    }

    T operator*() const {
        return *ptr();
    }


Review comment (suggestion):
No. Make the main thread wait for watchdog to get the static pointers, thats it. You can use a condition_variable for that

Code after:
#pragma once

#include <string>
#include <typeindex>
#include <hyprlang.hpp>
#include "../debug/Log.hpp"
#include "../macros.hpp"
#include "ConfigManager.hpp"

template <typename T>
class CConfigValue {
  public:
    CConfigValue(const std::string& val) {
        const auto PVHYPRLANG = g_pConfigManager->getHyprlangConfigValuePtr(val);

        p_ = PVHYPRLANG->getDataStaticPtr();

#ifdef HYPRLAND_DEBUG
        // verify type
        const auto ANY  = PVHYPRLANG->getValue();
        const auto TYPE = std::type_index(ANY.type());

        // exceptions
        const bool STRINGEX = (typeid(T) == typeid(std::string) && TYPE == typeid(Hyprlang::STRING));
        const bool CUSTOMEX = (typeid(T) == typeid(Hyprlang::CUSTOMTYPE) && (TYPE == typeid(Hyprlang::CUSTOMTYPE*) || TYPE == typeid(void*) /* dunno why it does this? */));

        RASSERT(typeid(T) == TYPE || STRINGEX || CUSTOMEX, "Mismatched type in CConfigValue<T>, got {} but has {}", typeid(T).name(), TYPE.name());
#endif
    }

    T* ptr() const {
        return *(T* const*)p_;
    }

    T operator*() const {
        return *ptr();
    }

  private:
Repository: hyprwm/Hyprland
File: src/config/ConfigManager.cpp (C++)

Code before:

                } catch (std::bad_any_cast& e) { Debug::log(ERR, "Bad any_cast on value {} in descriptions", value); }
                return std::string{""};
            },
            data);
    };

    std::string json = std::format(R"#({{
    "value": "{}",
    "description": "{}",
    "type": {},
    "flags": {},
    "data": {{
        {}
    }}
}})#",
                                   value, description, (uint16_t)type, (uint32_t)flags, parseData());

    return json;
}

void CConfigManager::ensurePersistentWorkspacesPresent() {
    g_pCompositor->ensurePersistentWorkspacesPresent(m_vWorkspaceRules);
}

void CConfigManager::storeFloatingSize(const std::string& szClass, const std::string& szTitle, const Vector2D& size) {
    Debug::log(LOG, "storing floating size {}x{} for window {}::{}", size.x, size.y, szClass, szTitle);
    m_mStoredFloatingSizes[szClass + "|||" + szTitle] = size;
}

std::optional<Vector2D> CConfigManager::getStoredFloatingSize(const std::string& szClass, const std::string& szTitle) {
    const auto KEY = szClass + "|||" + szTitle;
    if (m_mStoredFloatingSizes.contains(KEY)) {
        Debug::log(LOG, "got stored size {}x{} for window {}::{}", m_mStoredFloatingSizes[KEY].x, m_mStoredFloatingSizes[KEY].y, szClass, szTitle);
        return m_mStoredFloatingSizes[KEY];
    }
    return std::nullopt;
}


Review comment (suggestion):
use a data struct that is constructed with a `PHLWINDOW` as a parameter, and implement std::hash for it. Then you can just store those in an unordered_map

Code after:
                    }

                } catch (std::bad_any_cast& e) { Debug::log(ERR, "Bad any_cast on value {} in descriptions", value); }
                return std::string{""};
            },
            data);
    };

    std::string json = std::format(R"#({{
    "value": "{}",
    "description": "{}",
    "type": {},
    "flags": {},
    "data": {{
        {}
    }}
}})#",
                                   value, description, (uint16_t)type, (uint32_t)flags, parseData());

    return json;
}

void CConfigManager::ensurePersistentWorkspacesPresent() {
    g_pCompositor->ensurePersistentWorkspacesPresent(m_vWorkspaceRules);
}

void CConfigManager::storeFloatingSize(PHLWINDOW window, const Vector2D& size) {
    Debug::log(LOG, "storing floating size {}x{} for window {}::{}", size.x, size.y, window->m_szClass, window->m_szTitle);
    SFloatCache id{window};
    m_mStoredFloatingSizes[id] = size;
}

std::optional<Vector2D> CConfigManager::getStoredFloatingSize(PHLWINDOW window) {
    SFloatCache id{window};
    if (m_mStoredFloatingSizes.contains(id)) {
        Debug::log(LOG, "got stored size {}x{} for window {}::{}", m_mStoredFloatingSizes[id].x, m_mStoredFloatingSizes[id].y, window->m_szClass, window->m_szTitle);
        return m_mStoredFloatingSizes[id];
    }
    return std::nullopt;
}
Repository: hyprwm/Hyprland
File: src/desktop/LayerSurface.cpp (C++)

Code before:
        return false;

    return !realPosition->isBeingAnimated() && !realSize->isBeingAnimated() && !alpha->isBeingAnimated();
}

int CLayerSurface::popupsCount() {
    if (!layerSurface || !mapped || fadingOut)
        return 0;

    int no = -1; // we have one dummy
    popupHead->breadthfirst([](WP<CPopup> p, void* data) { *(int*)data += 1; }, &no);
    return no;
}

MONITORID CLayerSurface::monitorID() {
    return monitor ? monitor->ID : MONITOR_INVALID;
}

pid_t CLayerSurface::getPID() {
    pid_t PID = -1;

    wl_client_get_credentials(layerSurface->surface->getResource()->resource()->client, &PID, nullptr, nullptr);

    return PID;
}


Review comment (bug):
needs a check for nulls here. This could crash if you call `hyprctl layers` when a layer is fading out, I believe.

Code after:
        return false;

    return !realPosition->isBeingAnimated() && !realSize->isBeingAnimated() && !alpha->isBeingAnimated();
}

int CLayerSurface::popupsCount() {
    if (!layerSurface || !mapped || fadingOut)
        return 0;

    int no = -1; // we have one dummy
    popupHead->breadthfirst([](WP<CPopup> p, void* data) { *(int*)data += 1; }, &no);
    return no;
}

MONITORID CLayerSurface::monitorID() {
    return monitor ? monitor->ID : MONITOR_INVALID;
}

pid_t CLayerSurface::getPID() {
    pid_t PID = -1;

    if (!layerSurface || !layerSurface->surface || !layerSurface->surface->getResource() || !layerSurface->surface->getResource()->resource() ||
        !layerSurface->surface->getResource()->resource()->client)
        return -1;

    wl_client_get_credentials(layerSurface->surface->getResource()->resource()->client, &PID, nullptr, nullptr);

    return PID;
}