Skip to content
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.

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

FIX Loading adapter honors offline mode #1976

Conversation

BenjaminBossan
Copy link
Member

@BenjaminBossan BenjaminBossan commented Jul 30, 2024

HF_HUB_OFFLINE=1 was not honored when trying to load an adapter. This is now fixed.

Resolves huggingface/transformers#31700.

HF_HUB_OFFLINE=1 was not honored when trying to load an adapter. This is
now fixed.
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@BenjaminBossan
Copy link
Member Author

I wanted to add a test for this but for some reason, it does not work. Inside the test, I set os.environ["HF_HUB_OFFLINE_MODE"] = "1" but the adapter is still downloaded fine, as if the env var were not set. I even tried to start a new subprocess with the adjusted env var but this still had no effect. Not sure what the issue is.

Running this test manually works:

def test_load_from_hub_then_offline_model():
    peft_model_id = "peft-internal-testing/gpt2-lora-random"
    base_model = AutoModelForCausalLM.from_pretrained("gpt2")
    PeftModel.from_pretrained(base_model, peft_model_id)

Without the fix, this test fails with HF_HUB_OFFLINE_MODE=1 and with the fix it passes (run it without first to ensure that the adapter is available locally).

@Wauplin
Copy link
Contributor

Wauplin commented Jul 30, 2024

Hi @BenjaminBossan, HF_HUB_OFFLINE is evaluated only once at runtime. This is why updating the environment variable for the test does not work. What we do in huggingface_hub is to define a offline context manager (here) and use it like this. Example:

        for offline_mode in OfflineSimulationMode:
            with offline(mode=offline_mode):
                with SoftTemporaryDirectory() as tmpdir:
                    with self.assertRaises(LocalEntryNotFoundError):
                        snapshot_download(self.repo_id, cache_dir=tmpdir)

if os.path.exists(os.path.join(path, SAFETENSORS_WEIGHTS_NAME)):
filename = os.path.join(path, SAFETENSORS_WEIGHTS_NAME)
use_safetensors = True
elif os.path.exists(os.path.join(path, WEIGHTS_NAME)):
filename = os.path.join(path, WEIGHTS_NAME)
use_safetensors = False
elif os.environ.get("HF_HUB_OFFLINE", 0) in {"1", "ON", "YES", "TRUE"}: # same check as in hfh
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
elif os.environ.get("HF_HUB_OFFLINE", 0) in {"1", "ON", "YES", "TRUE"}: # same check as in hfh
elif huggingface_hub.constants.HF_HUB_OFFLINE:

Better to let huggingface_hub handle the env variable parsing. Suggestion needs an additional import.

@BenjaminBossan
Copy link
Member Author

What we do in huggingface_hub is to define a offline context manager

Thanks for this. It looks like I cannot import this, as it's not part of the src, right? So I would have to copy the whole function over to PEFT to use the context, or is there an easier way?

@Wauplin
Copy link
Contributor

Wauplin commented Jul 30, 2024

Thanks for this. It looks like I cannot import this, as it's not part of the src, right? So I would have to copy the whole function over to PEFT to use the context, or is there an easier way?

There is unfortunately no easy way to import this no 😕 Best way is to duplicate it, though I'd recommend to copy a simplified version of it with only

        with patch("huggingface_hub.constants.HF_HUB_OFFLINE", True):
            reset_sessions()
            yield
        reset_sessions()

You don't need to test the cases with ConnectionError or TimeoutError for this PR.

@BenjaminBossan
Copy link
Member Author

Thanks, this works perfectly!

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for adding the small test :)

@BenjaminBossan BenjaminBossan merged commit f13d860 into huggingface:main Jul 30, 2024
14 checks passed
@BenjaminBossan BenjaminBossan deleted the fix-load-adapter-honors-offline-mode branch July 30, 2024 14:11
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.

Unable to load models with adapter weights in offline mode
3 participants