Skip to content

Patch for Cambricon MLUs test #1747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jun 6, 2024
Merged

Patch for Cambricon MLUs test #1747

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d853d39
support mlu device
huismiling Aug 24, 2023
f605d3c
Merge commit '6c44096c7b8d55a2ecf24be9bc68393467e1584a' into mlu-dev
huismiling Sep 18, 2023
e0cfce6
merge from main
huismiling Apr 29, 2024
4f462b8
rollback
huismiling Apr 29, 2024
2fbdb33
up
huismiling Apr 29, 2024
3b2b8d6
add version check for mlu
huismiling Apr 30, 2024
2d0ef7c
better accelerate version check for mlu device
huismiling May 6, 2024
0965469
fix error with make style
huismiling May 7, 2024
4447fe2
patch for MLUs pytest
huismiling May 20, 2024
3e8e62b
Merge remote-tracking branch 'hf_peft/main' into mlu-dev
huismiling May 20, 2024
99965ac
make style for MLUs
huismiling May 22, 2024
83eaba2
rollback to torch.float16 for MLU test.
huismiling May 31, 2024
ad14bfe
skip test_merge_layers_fp16 for pt2.1 & cpu device
huismiling Jun 5, 2024
2b1ad33
fix err for test_merge_layers_fp16
huismiling Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions tests/test_custom_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,9 @@ def test_disable_adapters_with_merging(self, test_name, model_id, config_cls, co
# check that there is a difference in results after training
assert not torch.allclose(outputs_before, outputs_after, atol=atol, rtol=rtol)

if self.torch_device in ["mlu"] and model_id in ["Conv2d"]:
atol, rtol = 1e-3, 1e-2 # MLU

# unmerged or merged should make no difference
assert torch.allclose(outputs_after, outputs_unmerged, atol=atol, rtol=rtol)

Expand Down
3 changes: 3 additions & 0 deletions tests/test_lora_megatron.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@

from peft import LoraConfig, PeftModel, get_peft_model, get_peft_model_state_dict

from .testing_utils import require_torch_gpu


def is_megatron_available() -> bool:
return importlib.util.find_spec("megatron") is not None
Expand Down Expand Up @@ -93,6 +95,7 @@ def forward(self, input):
x = self.lm_head(x)[0]
return x

@require_torch_gpu
class TestMegatronLora(unittest.TestCase):
def setUp(self):
initialize_model_parallel(1, 1)
Expand Down
15 changes: 12 additions & 3 deletions tests/testing_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import torch
import yaml
from diffusers import StableDiffusionPipeline
from packaging import version

from peft import (
AdaLoraConfig,
Expand Down Expand Up @@ -464,13 +465,16 @@ def _test_merge_layers_fp16(self, model_id, config_cls, config_kwargs):
if ("gpt2" in model_id.lower()) and (config_cls != LoraConfig):
self.skipTest("Merging GPT2 adapters not supported for IA³ (yet)")

if (self.torch_device in ["cpu"]) and (version.parse(torch.__version__) <= version.parse(2.1)):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (self.torch_device in ["cpu"]) and (version.parse(torch.__version__) <= version.parse(2.1)):
if (self.torch_device in ["cpu"]) and (version.parse(torch.__version__) <= version.parse("2.1")):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for that. Fixed !

self.skipTest("PyTorch 2.1 not supported for Half of addmm_impl_cpu_ ")

model = self.transformers_class.from_pretrained(model_id, torch_dtype=torch.float16)
config = config_cls(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

torch_dtype=torch.float16 leads to an error.
RuntimeError: "addmm_impl_cpu_" not implemented for 'Half'

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I cannot replicate this, whether with or without GPU. The idea of this test is exactly to check that this error does not occur with fp16, so not using this dtype is counter-productive. Is this only occurring with MLU devices?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reproduction code is as follows.
The issue can be reproduced using PyTorch 2.1, but it executes normally with PyTorch 2.3.

import torch
a = torch.rand(4,4).to(torch.float16)
b = torch.rand(4,4).to(torch.float16)
a @ b

Copy link
Member

Choose a reason for hiding this comment

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

Okay, so instead of changing the dtype, how about skipping the test if an old pytorch version is detected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, maybe we can use fp16 with pt>=2.3, and fp32 with pt<2.3 ?

Copy link
Member

Choose a reason for hiding this comment

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

We really don't need to test merging with fp32 here, as it's tested extensively in other tests. This test is very specifically for merging with fp16, so if we don't use fp16, we can skip it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha, Got it! I will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I found that it is the device "cpu" leads error.
When device is changed to self.torch_device as I fixed, MLU tests is OK with torch.float16 .
@BenjaminBossan Will this test use "cpu" device? If not, it isn't need to skip test for pt2.1 .

peft/tests/testing_common.py

Line 473 in cb0bf07

model = model.to(device="cpu", dtype=torch.float16)

Copy link
Member

Choose a reason for hiding this comment

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

So IIRC, there is an error when using CPU + float16 + old PyTorch. If we change either of those variables, there is no error. On CI, we have a new PyTorch version, so it passes, despite using CPU.

If we switch to self.torch_device, it depends, because the device is auto-inferred based on what devices are available. So on our CI, this would still be CPU. On yours, it might not, but then we don't really test what was the original intent, namely that float16 works on CPU.

I assume this fails on your CI because it uses an older PyTorch version. This is why I suggested to just skip the test with older PyTorch versions. If you want, you could add a specific test for merging float16 with MLU, which would be skipped if the device is not available.

Copy link
Contributor Author

@huismiling huismiling Jun 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@BenjaminBossan
Got it! Skipping test_merge_layers_fp16 for pt2.1 when cpu device, this should be OK.

base_model_name_or_path=model_id,
**config_kwargs,
)
model = get_peft_model(model, config)
model = model.to(device="cpu", dtype=torch.float16)
model = model.to(device=self.torch_device, dtype=torch.float16)

model.eval()

Expand Down Expand Up @@ -561,6 +565,8 @@ def _test_merge_layers(self, model_id, config_cls, config_kwargs):
logits_merged_unloaded = model(**dummy_input)[0]

atol, rtol = 1e-4, 1e-4
if self.torch_device in ["mlu"]:
atol, rtol = 1e-3, 1e-3 # MLU
if (config.peft_type == "IA3") and (model_id == "Conv2d"):
# for some reason, the IA³ Conv2d introduces a larger error
atol, rtol = 0.3, 0.01
Expand Down Expand Up @@ -689,16 +695,19 @@ def _test_safe_merge(self, model_id, config_cls, config_kwargs):
model = get_peft_model(model, config).eval()
logits_peft = model(**inputs)[0]

atol, rtol = 1e-6, 1e-6 # default
# Initializing with LN tuning cannot be configured to change the outputs (unlike init_lora_weights=False)
if not issubclass(config_cls, LNTuningConfig):
# sanity check that the logits are different
assert not torch.allclose(logits_base, logits_peft, atol=1e-6, rtol=1e-6)
assert not torch.allclose(logits_base, logits_peft, atol=atol, rtol=rtol)

model_unloaded = model.merge_and_unload(safe_merge=True)
logits_unloaded = model_unloaded(**inputs)[0]

if self.torch_device in ["mlu"]:
atol, rtol = 1e-3, 1e-3 # MLU
# check that the logits are the same after unloading
assert torch.allclose(logits_peft, logits_unloaded, atol=1e-6, rtol=1e-6)
assert torch.allclose(logits_peft, logits_unloaded, atol=atol, rtol=rtol)

def _test_mixed_adapter_batches(self, model_id, config_cls, config_kwargs):
# Test for mixing different adapters in a single batch by passing the adapter_names argument
Expand Down
Loading