ronantakizawa/codereview-bench huggingface dataset:
Repository: llvm/llvm-project
File: llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (C++)

Code before:
    Cmp->setPredicate(UnsignedPred);
    Changed = true;
  }

  if (ConstantRange::areInsensitiveToSignednessOfICmpPredicate(CR1, CR2)) {
    Cmp->setSameSign();
    Changed = true;
  }

  return Changed;
}

/// See if LazyValueInfo's ability to exploit edge conditions or range
/// information is sufficient to prove this comparison. Even for local
/// conditions, this can sometimes prove conditions instcombine can't by
/// exploiting range information.
static bool constantFoldCmp(CmpInst *Cmp, LazyValueInfo *LVI) {
  Value *Op0 = Cmp->getOperand(0);
  Value *Op1 = Cmp->getOperand(1);
  Constant *Res = LVI->getPredicateAt(Cmp->getPredicate(), Op0, Op1, Cmp,
                                      /*UseBlockValue=*/true);
  if (!Res)
    return false;

  bool Changed = false;
  Cmp->replaceUsesWithIf(Res, [&Changed](Use &use) {
    if (auto *intrin = dyn_cast<IntrinsicInst>(use.getUser()))
      if (intrin->isAssumeLikeIntrinsic())
        return false;
    Changed = true;
    return true;
  });
  if (Cmp->use_empty()) {
    Cmp->eraseFromParent();
    Changed = true;
  }

  if (Changed)
    ++NumCmps;

  return Changed;
}

static bool processCmp(CmpInst *Cmp, LazyValueInfo *LVI) {
  if (constantFoldCmp(Cmp, LVI))
    return true;

  if (auto *ICmp = dyn_cast<ICmpInst>(Cmp))
    if (processICmp(ICmp, LVI))
      return true;


Review comment (suggestion):
```suggestion
  Cmp->replaceUsesWithIf(Res, [&Changed](Use &U) {
```

Code after:
    Cmp->setPredicate(UnsignedPred);
    Changed = true;
  }

  if (ConstantRange::areInsensitiveToSignednessOfICmpPredicate(CR1, CR2)) {
    Cmp->setSameSign();
    Changed = true;
  }

  return Changed;
}

/// See if LazyValueInfo's ability to exploit edge conditions or range
/// information is sufficient to prove this comparison. Even for local
/// conditions, this can sometimes prove conditions instcombine can't by
/// exploiting range information.
static bool constantFoldCmp(CmpInst *Cmp, LazyValueInfo *LVI) {
  Value *Op0 = Cmp->getOperand(0);
  Value *Op1 = Cmp->getOperand(1);
  Constant *Res = LVI->getPredicateAt(Cmp->getPredicate(), Op0, Op1, Cmp,
                                      /*UseBlockValue=*/true);
  if (!Res)
    return false;

  bool Changed = Cmp->replaceUsesWithIf(
      Res, [](Use &U) { return !isa<AssumeInst>(U.getUser()); });
  if (Cmp->use_empty()) {
    Cmp->eraseFromParent();
    Changed = true;
  }

  if (Changed)
    ++NumCmps;

  return Changed;
}

static bool processCmp(CmpInst *Cmp, LazyValueInfo *LVI) {
  if (constantFoldCmp(Cmp, LVI))
    return true;

  if (auto *ICmp = dyn_cast<ICmpInst>(Cmp))
    if (processICmp(ICmp, LVI))
      return true;

  return false;
}

/// Simplify a switch instruction by removing cases which can never fire. If the
/// uselessness of a case could be determined locally then constant propagation
/// would already have figured it out. Instead, walk the predecessors and
Repository: llvm/llvm-project
File: clang/test/APINotes/objc_designated_init_protocol.m (Objective-C)

Code before:
// RUN: rm -rf %t && mkdir -p %t
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache -fapinotes-modules -Wno-private-module -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -verify

// Regression test for APINotesWriter::addObjCMethod asserting when a protocol
// method is annotated with DesignatedInit: true in an .apinotes file.
// The writer was reconstructing the ContextTableKey with a hardcoded
// ContextKind::ObjCClass, causing the Contexts lookup to fail when the
// context was actually a protocol. Just importing the module is enough to
// trigger the writer path that previously crashed.

// expected-no-diagnostics

#import <SomeKit/SomeKit.h>

void useProtocol(id<InitializableProtocol> p) {}


Review comment (suggestion):
Could we dump the AST and add a FileCheck to make sure the attribute is properly applied? The other APINotes tests should have some examples.

Code after:
// RUN: rm -rf %t && mkdir -p %t
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache -fapinotes-modules -Wno-private-module -fsyntax-only -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s

// Regression test for APINotesWriter::addObjCMethod asserting when a protocol
// method is annotated with DesignatedInit: true in an .apinotes file.
// The writer was reconstructing the ContextTableKey with a hardcoded
// ContextKind::ObjCClass, causing the Contexts lookup to fail when the
// context was actually a protocol. Importing the module is sufficient to
// trigger the writer path that previously crashed.

// expected-no-diagnostics

#import <SomeKit/SomeKit.h>

id<InitializableProtocolDUMP> useProtocol(id<InitializableProtocolDUMP> p) {
  return [p initWithValue:0];
}
Repository: llvm/llvm-project
File: clang/lib/APINotes/APINotesFormat.h (C/C++)

Code before:
//===-- APINotesFormat.h - API Notes Format ---------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_LIB_APINOTES_APINOTESFORMAT_H
#define LLVM_CLANG_LIB_APINOTES_APINOTESFORMAT_H

#include "clang/APINotes/Types.h"
#include "llvm/ADT/PointerEmbeddedInt.h"
#include "llvm/Bitcode/BitcodeConvenience.h"

namespace clang {
namespace api_notes {
/// Magic number for API notes files.
const unsigned char API_NOTES_SIGNATURE[] = {0xE2, 0x9C, 0xA8, 0x01};

/// API notes file major version number.
const uint16_t VERSION_MAJOR = 0;

/// API notes file minor version number.
///
/// When the format changes IN ANY WAY, this number should be incremented.

Review comment (suggestion):
```suggestion
//===----------------------------------------------------------------------===//
```

The new LLVM style has dropped these headers.

Code after:
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_LIB_APINOTES_APINOTESFORMAT_H
#define LLVM_CLANG_LIB_APINOTES_APINOTESFORMAT_H

#include "clang/APINotes/Types.h"
#include "llvm/ADT/PointerEmbeddedInt.h"
#include "llvm/Bitcode/BitcodeConvenience.h"

namespace clang {
namespace api_notes {
/// Magic number for API notes files.
const unsigned char API_NOTES_SIGNATURE[] = {0xE2, 0x9C, 0xA8, 0x01};

/// API notes file major version number.
const uint16_t VERSION_MAJOR = 0;

/// API notes file minor version number.
///
/// When the format changes IN ANY WAY, this number should be incremented.
Repository: llvm/llvm-project
File: clang/test/CIR/CodeGen/c89-implicit-int.c (C)

Code before:
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c89 -emit-cir %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR

// Implicit int return type.
test = 0;
// CIR: cir.global external @test = #cir.int<0> : !s32i
func (void) {
// CIR: cir.func {{.*}} @func() -> !s32i
  return 0;
}


Review comment (suggestion):
I think the FileCheck lines should be moved after the closing `}` (as in `clang/test/CIR/CodeGen/basic.c`).

Code after:
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c89 -emit-cir %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR

// Implicit int return type.
test = 0;
func (void) {
  return 0;
}

// CIR: cir.global external @test = #cir.int<0> : !s32i
// CIR: cir.func {{.*}} @func() -> !s32i
Repository: lm-sys/FastChat
File: fastchat/serve/inference.py (Python)

Code before:
"""Inference for FastChat models."""
import abc
from typing import Optional
import warnings

import torch
try:
    from transformers import AutoTokenizer, AutoModelForCausalLM, LlamaTokenizer, LlamaForCausalLM, AutoModel, LlamaForCausalLM, AutoConfig 
except ImportError:
    from transformers import AutoTokenizer, AutoModelForCausalLM, LLaMATokenizer, LLamaForCausalLM, AutoModel

from fastchat.conversation import conv_templates, get_default_conv_template, SeparatorStyle
from fastchat.serve.compression import  load_compress_model
from fastchat.serve.monkey_patch_non_inplace import replace_llama_attn_with_non_inplace_operations
from fastchat.serve.serve_chatglm import chatglm_generate_stream



def raise_warning_for_old_weights(model_path, model):
    if "vicuna" in model_path.lower():
        try:
            is_vicuna = isinstance(model, LlamaForCausalLM)
        except Exception:
            is_vicuna = isinstance(model, LLamaForCausalLM)
        if is_vicuna and model.model.vocab_size > 32000:
            warnings.warn(
                "\nYou are probably using the old Vicuna-v0 model, "
                "which will generate unexpected results with the "
                "current fschat.\nYou can try one of the following methods:\n"
                "1. Upgrade your weights to the new Vicuna-v1.1: https://github.com/lm-sys/FastChat#vicuna-weights.\n"
                "2. Use the old conversation template by `python3 -m fastchat.serve.cli --model-path /path/to/vicuna-v0 --conv-template conv_one_shot`\n"
                "3. Downgrade fschat to fschat==0.1.10 (Not recommonded).\n")


def compute_skip_echo_len(model_name, conv, prompt):
    model_name = model_name.lower()
    if "chatglm" in model_name:
        skip_echo_len = len(conv.messages[-2][1]) + 1

Review comment (suggestion):
```suggestion
from fastchat.serve.compression import load_compress_model
```

Code after:
"""Inference for FastChat models."""
import abc
from typing import Optional
import warnings

import torch

try:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LlamaTokenizer,
        LlamaForCausalLM,
        AutoModel,
        AutoModelForSeq2SeqLM,
        AutoConfig,
    )
except ImportError:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LLaMATokenizer,
        LLamaForCausalLM,
        AutoModel,
        AutoModelForSeq2SeqLM,
        AutoConfig,
    )

from fastchat.conversation import (
    conv_templates,
    get_default_conv_template,
    compute_skip_echo_len,
    SeparatorStyle,
)
from fastchat.serve.compression import load_compress_model
from fastchat.serve.monkey_patch_non_inplace import (
    replace_llama_attn_with_non_inplace_operations,
)
Repository: lm-sys/FastChat
File: fastchat/serve/model_worker.py (Python)

Code before:
"""
A model worker executes the model.
"""
import os
import argparse
import asyncio
import dataclasses
import logging
import json
import time
from typing import List, Union
import threading
import uuid

from fastapi import FastAPI, Request, BackgroundTasks
from fastapi.responses import StreamingResponse
import requests

try:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LlamaTokenizer,
        AutoModel,
    )
except ImportError:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LLaMATokenizer,
        AutoModel,
    )

Review comment (suggestion):
```suggestion
import argparse
import asyncio
import dataclasses
import os
```

Code after:
"""
A model worker executes the model.
"""
import argparse
import asyncio
import dataclasses
import logging
import json
import os
import time
from typing import List, Union
import threading
import uuid

from fastapi import FastAPI, Request, BackgroundTasks
from fastapi.responses import StreamingResponse
import requests

try:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LlamaTokenizer,
        AutoModel,
    )
except ImportError:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LLaMATokenizer,
        AutoModel,
    )
Repository: lm-sys/FastChat
File: fastchat/serve/model_worker.py (Python)

Code before:
"""
A model worker executes the model.
"""
import argparse
import asyncio
import dataclasses
import os
import logging
import json
import time
from typing import List, Union
import threading
import uuid

from fastapi import FastAPI, Request, BackgroundTasks
from fastapi.responses import StreamingResponse
import requests

try:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LlamaTokenizer,
        AutoModel,
    )
except ImportError:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LLaMATokenizer,
        AutoModel,
    )
import torch
import uvicorn

Review comment (suggestion):
```suggestion
import logging
import json
import os
```

Code after:
"""
A model worker executes the model.
"""
import argparse
import asyncio
import dataclasses
import logging
import json
import os
import time
from typing import List, Union
import threading
import uuid

from fastapi import FastAPI, Request, BackgroundTasks
from fastapi.responses import StreamingResponse
import requests

try:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LlamaTokenizer,
        AutoModel,
    )
except ImportError:
    from transformers import (
        AutoTokenizer,
        AutoModelForCausalLM,
        LLaMATokenizer,
        AutoModel,
    )
import torch
import uvicorn
Repository: lm-sys/FastChat
File: fastchat/conversation.py (Python)

Code before:
register_conv_template(
    Conversation(
        name="claude",
        system="",
        roles=("Human", "Assistant"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.ADD_COLON_SINGLE,
        sep="\n\n",
    )
)

# MPT default template
register_conv_template(
    Conversation(
        name="mpt",
        system="""
system
            - You are a helpful assistant chatbot trained by MosaicML.
            - You answer questions.
            - You are excited to be able to help the user, but will refuse to do anything that could be considered harmful to the user.
            - You are more than just an information source, you are also able to write poetry, short stories, and make jokes.
。
            """,
        roles=("
user", "
assistant"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.ADD_COLON_SINGLE,
        sep="
",
    )
)

if __name__ == "__main__":
    conv = get_conv_template("vicuna_v1.1")
    conv.append_message(conv.roles[0], "Hello!")
    conv.append_message(conv.roles[1], "Hi!")
    conv.append_message(conv.roles[0], "How are you?")
    conv.append_message(conv.roles[1], None)
    print(conv.get_prompt())


Review comment (suggestion):
I noticed that they used "\n" instead of ":" (colon) in their official demo.
Can we change it to match their official demo?
https://huggingface.co/spaces/mosaicml/mpt-7b-chat/blob/22728d16cb77d5f7ae2c2598b82b471c4d6915a0/app.py#L66-L67

Code after:
        roles=("user", "assistant"),
        messages=(),
        offset=0,
        sep_style=None,
        sep=None,
    )
)

# Claude default template
register_conv_template(
    Conversation(
        name="claude",
        system="",
        roles=("Human", "Assistant"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.ADD_COLON_SINGLE,
        sep="\n\n",
    )
)

# MPT default template
register_conv_template(
    Conversation(
        name="mpt",
        system="""
system
- You are a helpful assistant chatbot trained by MosaicML.
- You answer questions.
- You are excited to be able to help the user, but will refuse to do anything that could be considered harmful to the user.
- You are more than just an information source, you are also able to write poetry, short stories, and make jokes.
""",
        roles=("
user", "
assistant"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.NEW_LINE,
        sep="
",
        stop_token_ids=[50278, 0],
    )
)

if __name__ == "__main__":
    conv = get_conv_template("vicuna_v1.1")
    conv.append_message(conv.roles[0], "Hello!")
    conv.append_message(conv.roles[1], "Hi!")
    conv.append_message(conv.roles[0], "How are you?")
    conv.append_message(conv.roles[1], None)
    print(conv.get_prompt())
Repository: lm-sys/FastChat
File: fastchat/conversation.py (Python)

Code before:
        sep_style=None,
        sep=None,
    )
)

# BiLLa default template
register_conv_template(
    Conversation(
        name="billa",
        system="",
        roles=("Human", "Assistant"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.BILLA,
        sep="\n",
        stop_str="Human:",
    )
)

# Snoozy default template
# Reference: https://github.com/nomic-ai/gpt4all/blob/d4861030b778da6db59d21d2927a4aba4f9f1f43/gpt4all-bindings/python/gpt4all/gpt4all.py#L232
register_conv_template(
    Conversation(
        name="snoozy",
        system="The prompt below is a question to answer, a task to complete, or a conversation to respond to; decide which and write an appropriate response.\n",
        roles=("user", "assistant"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.NEW_LINE,
        sep="\n",
    )
)

if __name__ == "__main__":
    conv = get_conv_template("vicuna_v1.1")
    conv.append_message(conv.roles[0], "Hello!")
    conv.append_message(conv.roles[1], "Hi!")
    conv.append_message(conv.roles[0], "How are you?")
    conv.append_message(conv.roles[1], None)
    print(conv.get_prompt())


Review comment (bug):
Please try to strictly follow https://github.com/nomic-ai/gpt4all/blob/d4861030b778da6db59d21d2927a4aba4f9f1f43/gpt4all-bindings/python/gpt4all/gpt4all.py#L232.

For example, they use "### Instruction"/"### Response" instead of "user" / "assistant"

You can use `python3 -m fastchat.serve.cli --model XXX --debug` to print the prompts and check them.

Code after:
# RedPajama INCITE default template
register_conv_template(
    Conversation(
        name="redpajama-incite",
        system="",
        roles=("<human>", "<bot>"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.ADD_COLON_SINGLE,
        sep="\n",
        stop_str="<human>",
    )
)

# h2oGPT default template
register_conv_template(
    Conversation(
        name="h2ogpt",
        system="",
        roles=("
", "
"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.NO_COLON_SINGLE,
        sep="
",
    )
)

# Snoozy default template
# Reference: https://github.com/nomic-ai/gpt4all/blob/d4861030b778da6db59d21d2927a4aba4f9f1f43/gpt4all-bindings/python/gpt4all/gpt4all.py#L232
register_conv_template(
    Conversation(
        name="snoozy",
        system="### Instruction: The prompt below is a question to answer, a task to complete, or a conversation to respond to; decide which and write an appropriate response.",
        roles=("### Prompt", "### Response"),
        messages=(),
        offset=0,
        sep_style=SeparatorStyle.ADD_COLON_SINGLE,
        sep="\n",
        stop_str="###",
    )
)

if __name__ == "__main__":
    conv = get_conv_template("vicuna_v1.1")
    conv.append_message(conv.roles[0], "Hello!")
    conv.append_message(conv.roles[1], "Hi!")
    conv.append_message(conv.roles[0], "How are you?")
    conv.append_message(conv.roles[1], None)
    print(conv.get_prompt())
Repository: lm-sys/FastChat
File: fastchat/train/train_lora.py (Python)

Code before:
        padding_side="right",
        use_fast=False,
    )
    tokenizer.pad_token = tokenizer.unk_token

    data_module = make_supervised_data_module(tokenizer=tokenizer, data_args=data_args)
    trainer = Trainer(
        model=model, tokenizer=tokenizer, args=training_args, **data_module
    )

    model.config.use_cache = False

    if list(pathlib.Path(training_args.output_dir).glob("checkpoint-*")):
        trainer.train(resume_from_checkpoint=True)
    else:
        trainer.train()
    trainer.save_state()

    # Save states. Weights might be a placeholder in zero3 and need a gather
    state_dict = get_peft_state_maybe_zero_3(
        model.named_parameters(), lora_args.lora_bias
    )
    if training_args.local_rank == 0:
        model.save_pretrained(training_args.output_dir, state_dict=state_dict)


if __name__ == "__main__":
    train()


Review comment (suggestion):
Seems like this one gathers the weight of the whole model, while we'd hope only to gather that of the LoRA(For instance, when training a 30B model with 2x40GB A100, gathering the whole 30B model needs at least 60GB memory, resulting in an OOM).

Code after:
        padding_side="right",
        use_fast=False,
    )
    tokenizer.pad_token = tokenizer.unk_token

    data_module = make_supervised_data_module(tokenizer=tokenizer, data_args=data_args)
    trainer = Trainer(
        model=model, tokenizer=tokenizer, args=training_args, **data_module
    )

    model.config.use_cache = False

    if list(pathlib.Path(training_args.output_dir).glob("checkpoint-*")):
        trainer.train(resume_from_checkpoint=True)
    else:
        trainer.train()
    trainer.save_state()

     # check if zero3 mode enabled
    if trainer.hf_deepspeed_config_orig.is_zero3():
        # use deepspeed engine internal function to gather state dict
        # state_dict_zero3 contains whole parameters of base and lora adapters
        # we will not extract lora parameters since peft save_pretrained will do that
        # https://github.com/huggingface/peft/blob/3714aa2fff158fdfa637b2b65952580801d890b2/src/peft/peft_model.py#L125
        # https://github.com/huggingface/peft/blob/3714aa2fff158fdfa637b2b65952580801d890b2/src/peft/utils/save_and_load.py#L19
        state_dict_zero3 = trainer.model_wrapped._zero3_consolidated_16bit_state_dict()
        if training_args.local_rank == 0:
            state_dict = state_dict_zero3
    else:
        # in other mode we use original code from fastchat team, to make sure our change is minimum
        state_dict = get_peft_state_maybe_zero_3(
        model.named_parameters(), lora_args.lora_bias
        )   
        
    if training_args.local_rank == 0:
        model.save_pretrained(training_args.output_dir, state_dict=state_dict)


if __name__ == "__main__":
    train()