Add sample unit tests

[tedhtchang]

Changes:

1. Adds unit tests for utilts/data_type_utils module
2. Enables unit test framework in the workflow for every PR

Closes #5

Verify in a virtual env:

git clone https://github.com/tedhtchang/fms-hf-tuning -b add_sample_unit_test
cd fms-hf-tuning
pip install -r setup_requirement.txt
tox run -e py

Should see something like:

============================================================================================================================= test session starts ==============================================================================================================================
platform darwin -- Python 3.11.7, pytest-8.0.2, pluggy-1.4.0
cachedir: .tox/py/.pytest_cache
rootdir: /Users/tedchang/pub_git/fms-hf-tuning
collected 3 items

tests/test_data_type_utils.py ...                                                                                                                                                                                                                                        [100%]

============================================================================================================================== 3 passed in 0.97s ===============================================================================================================================

[Ssukriti]

the black formatter checks are failing

[tedhtchang]

the black formatter checks are failing

Thanks
Installed pre-commit hook locally to make sure formatting is run automatically for future PR.

[jbusche]

I tried it on my laptop and on a Red Hat server... both looked good:

Mac Laptop:

====================================================================== test session starts ======================================================================
platform darwin -- Python 3.11.7, pytest-8.0.2, pluggy-1.4.0
cachedir: .tox/py/.pytest_cache
rootdir: /Users/jamesbusche/projects/RESEARCH/TED/fms-hf-tuning
collected 3 items                                                                                                                                               

tests/test_data_type_utils.py ...                                                                                                                         [100%]
======================================================================= 3 passed in 4.38s

Red Hat Server:

====================================================================== test session starts ======================================================================
platform linux -- Python 3.11.5, pytest-8.0.2, pluggy-1.4.0
cachedir: .tox/py/.pytest_cache
rootdir: /root/JIM/fms-hf-tuning
collected 3 items                                                                                                                                               

tests/test_data_type_utils.py ...                                                                                                                         [100%]

======================================================================= 3 passed in 1.42s

[Ssukriti]

@joe.olson@ibm.com
@olson-ibm

[fabianlim]

im not sure if this is a good function to test. THis is a function that deals with configs, but for such things, its better to defer to huggingface, because changes come in very fast

As an example, this function will fail if auto is passed in, because it is not an "actual" torch dtype

however, it is a valid flag, as seen in this code https://github.com/huggingface/transformers/blob/0ad770c3733f9478a8d9d0bc18cc6143877b47a2/src/transformers/modeling_utils.py#L3317-L3339

[tedhtchang]

I am testing based on my knowledge what those functions will return. If auto is a valid input then our functions should have logic to handle it. Then we can pass in auto to the unit test to make sure our functions will not get ValueError.

>>> from tuning.utils import data_type_utils
>>>  data_type_utils.str_to_torch_dtype("auto")
 ValueError: Unrecognized data type of a torch.Tensor

[fabianlim]

ok i see, I think i misunderstood https://github.com/huggingface/transformers/blob/0ad770c3733f9478a8d9d0bc18cc6143877b47a2/src/transformers/modeling_utils.py#L2707-L2727,

the only str value it will take is auto, the rest relies on it to be a torch dtype. So i think its a valid function.

If think omitting auto is a design decision. either way is fine.,

[fabianlim]

Sorry an update to my previous comment

the only str value it will take is auto, the rest relies on it to be a torch dtype. So i think its a valid function.

It turns out that trl has a ModelConfig dataclass which handles torch_dtype. I think its better to rely on that

https://github.com/huggingface/trl/blob/3bd02380c7e39cae2ec5013f27907b30bc4ce6bf/trl/trainer/model_config.py#L21

[tedhtchang]

@fabianlim Are you suggesting

1. These functions need to to handle auto? If so create an issue.
2. Or we need to add more test case to test these functions? If so, what additional input to the functions do I need to test and what is the expected output of the functions do I need to assert.

[Ssukriti]

@fabianlim @tedhtchang to re-iterate the purpose of this PR is to get some basic unit tests into fms-tuning to get started with tests in our CI/CD that automatically run with every pull request to help us catch any code changes that are breaking functionality.

The unit test being written here is to validate the function in library https://github.com/foundation-model-stack/fms-hf-tuning/blob/main/tuning/utils/data_type_utils.py#L11 which is a custom function written by us.

Any changes that need to be made to the underlying capabilities are NOT in scope of this PR. This PR is just testing what we support.

we do not currently support "auto" str to from_pretrained https://github.com/foundation-model-stack/fms-hf-tuning/blob/5a0cf5c1f2649d38f75ed02c2153c9d1850b20c7/tuning/sft_trainer.py#L129C21-L129C36

• that might be a good feature to add as a separate issue. (see line 129)
• @fabianlim when you see gaps like that, would advise to create an issue and tag me - you can also create one in this repo. when that feature is added, the corresponding unit test will be requested in that PR

on your point of using ModelConfig Fabian - TRL does the exact same thing with the torch_dtype = (
model_config.torch_dtype
if model_config.torch_dtype in ["auto", None]
else getattr(torch, model_config.torch_dtype)
https://github.com/huggingface/trl/blob/3bd02380c7e39cae2ec5013f27907b30bc4ce6bf/examples/scripts/sft.py#L74C2-L77C54
They use the modelConfig just to accept the parameter from user, we use our own config

fms-hf-tuning/tuning/config/configs.py

Line 20 in 5a0cf5c

class ModelArguments:

for that. But internally it has to be converted to torch dtype and if it is "auto" -> it needs to remain str (the "auto" part is missing in our codebase today)

The discussions on how to improve code should be separated from adding tests for existing code and getting a framework ready

[Ssukriti]

@tedhtchang as far as these unit go, they look great , thank you! we do not need to test each and every torch dtype . If conversion works for some , it will work for others as it is using same codepath. So i will add a separate comment on reducing the number of tests - remember every parameter value you pass is like another test - CI/CD tests also need to run fast.
So we need CODE coverage and pass parameters that would go through a separate codepath while testing - example if "float32" were handled differently from "float64" then it makes sense to have 2 tests. Here all string to torch dtype is happening with 1 codepath - so we need not have 20+ tests for one codepath

[Ssukriti]

some small minor changes, comments and test for ValueError

when these changes are addressed, PR will be merged

[Ssukriti]

thank you!

[Ssukriti]

Excellent work, thank you!

[tedhtchang]

Excellent work, thank you!

Thanks for the review.