Add config_utils tests

[aluu317]

Description of the change

Add unit tests for the config_utils with 100% test coverage

Related issue number

Part of #75

How to verify the PR

Was the PR tested

• I have added >=1 unit test(s) for every new method I have added.
• I have ensured all unit tests pass

[Abhishek-TAMU]

Thank you for the PR!
Just curious to know one thing:

In the test function test_get_hf_peft_config_returns_PT_config_correctly, below code checks and raises error tokenizer_name_or_path cannot be None.

    tuning_config = peft_config.PromptTuningConfig(prompt_tuning_init="TEXT")
    with pytest.raises(ValueError) as err:
        config_utils.get_hf_peft_config(None, tuning_config, None)
        assert "tokenizer_name_or_path can't be None" in err.value

As this check and raising of error is done by PromptTuningConfig Class of peft library and not by our code, so was just curious if we comment this code do we still get 100% coverage in file tuning/utils/config_utils.py?

Apart from this doubt, everything looks good to me! Thanks!

[aluu317]

Yea, you're right @Abhishek-TAMU . That test doesn't increase test coverage for tuning/utils/config_utils.py. I just wanted to test that the value None for tokenizer_name_or_path was passed down to PromptTuningConfig init and post_init properly and if there's error/exception, we are able to catch it in our utils.

Additional testing wasn't necessary but I'm sure wouldn't hurt

[anhuong]

Amazing tests Angel, thank you so much for adding them! Have a few small comments on things to change, such as your test methods are testing such good and varied use cases. But since they are testing different functionality, you should separate out each test case, each test method should test a specific aspect/use-case of the function or method. Use long and descriptive names for testing functions to show what you are testing.

[anhuong]

Another test could be, what happens if a tokenizer is passed? Since tokenizer_name_or_path is by default set to model_name_or_path, then this value will always get passed when it's run in sft_trainer.py. I believe tokenizer is used in PromptTuning but would be ignored for LoRA?

[anhuong]

This is the only comment that I think wasn't addressed, what do you think about adding a test where you run with lora and pass in a tokenizer (which we expect to be ignored)?

[anhuong]

Interesting! So that means others like prompt_tuning_init_text which is also only used if prompt_tuning_init is TEXT would also become None?

[aluu317]

Updated the function call to be more readable. The function call is

config = config_utils.get_hf_peft_config(task_type=None, tuning_config=tuning_config, tokenizer_name_or_path=None)

So the tokenizer_name_or_path is allowed to be None only when prompt_tuning_init is type RANDOM. When prompt_tuning_init is TEXT, you cannot pass None for tokenizer. It raises an exception. Reference

Other fields are not affected.

[anhuong]

LGTM, many thanks Angel! Love having more test coverage