-
Notifications
You must be signed in to change notification settings - Fork 45
[Tests]: Adding dummy causal models for testing in regular CI run #427
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
"hpcai-tech/grok-1", | ||
] | ||
|
||
test_dummy_model_configs = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this outside this file? may be we can maintain a CSV file for better readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, made a json file for dummy configs.
"hpcai-tech/grok-1", | ||
] | ||
|
||
test_dummy_model_configs = [ | ||
# model_name, model_type, max_position_embeddings, num_hidden_layers, num_attention_heads, hidden_size, intermediate_size, vocab_size, additional_params | ||
("TinyLlama/TinyLlama-1.1B-Chat-v1.0", "llama", 128, 1, 2, 64, 256, 32000, {"num_key_value_heads": 1}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we following any criteria for selecting these configs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No criteria, but some of the models has constraint for some of the config params with specific value.
|
||
if model_hf is None: | ||
model_hf, _ = load_causal_lm_model(model_config) | ||
model_hf_cb = copy.deepcopy(model_hf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because, later for CB testing we don't need to call the load_causal_lm_model
again. And it reduces the code for dummy model execution. If you don't want this, please let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need to have a copy of model_hf for CB?
@pytest.mark.cli | ||
@pytest.mark.parametrize("config", configs) | ||
def test_export_compile_execute_qnn_fb(mocker, config): | ||
# testing export -> compile -> infer with full_batch_size in QNN enviroment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in "enviroment"
@pytest.mark.qnn | ||
@pytest.mark.cli | ||
@pytest.mark.parametrize("config", configs) | ||
def test_export_compile_execute_qnn(mocker, config): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Both test_export_compile_execute_qnn and test_export_compile_execute_qnn_fb is currently having same configs right? Ideally in test_export_compile_execute_qnn we should be providing BS and in test_export_compile_execute_qnn_fb we should be providing FBS.
- Rename test_export_compile_execute_qnn_fb -> test_export_compile_execute_qnn_fbs for better readability
- Typo in 'enviroment'
tests/cloud/test_infer.py
Outdated
) | ||
check_infer(mocker=mocker, generation_len=20, **local_config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a vlm qnn test as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried adding this test but got error, so removed from the test.
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
Signed-off-by: Abukhoyer Shaik <[email protected]>
"TheBloke/TinyLlama-1.1B-Chat-v0.3-AWQ", | ||
} | ||
|
||
extrenal_models = {"hpcai-tech/grok-1"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Typo "extrenal_models"
else: | ||
config_dict[config["model_name"]] = AutoConfig.for_model( | ||
config["model_type"], | ||
max_position_embeddings=config["max_position_embeddings"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a get function instead of config[]? in case some params are not available it will throw a key error
config_objects = [] | ||
model_names = [] | ||
for name, config in model_config_dict.items(): | ||
if name in selected_model_names: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make sure the selected_model_names is a set and not list or tuple
trust_remote_code=model_name in extrenal_models, | ||
) | ||
else: | ||
torch.manual_seed(42) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you define this at the top?
|
||
if model_hf is None: | ||
model_hf, _ = load_causal_lm_model(model_config) | ||
model_hf_cb = copy.deepcopy(model_hf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need to have a copy of model_hf for CB?
assert os.path.isfile(os.path.join(os.path.dirname(qpc_path), "qconfig.json")) | ||
|
||
|
||
# Load the custom models configuration data from the JSON file | ||
with open("tests/transformers/models/custom_tiny_model_configs.json", "r") as f: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a better way to handle this? We can use a class or using a global variable to load the configs and keep it. Read the configs and model names from each test methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need to have a copy of model_hf for CB?
=> If we don't copy then we will have to load it again for CB testing as we cannot use the same model for CB and Non-CB testing.
Signed-off-by: Abukhoyer Shaik <[email protected]>
Purpose of this PR:
This update aims to reduce test execution time for causal language model inference. Previously, tests were run using full-scale models with one or two layers, which was inefficient and time-consuming. Refactoring CLI api testing for independent testing and redundant conftest code.
What’s Changed:
Introduced dummy models with significantly smaller configurations by adjusting parameters such as
max_position_embeddings, num_hidden_layers, num_attention_heads, hidden_size, intermediate_size, vocab_size and additional_params
.These lightweight models are used exclusively for testing purposes to ensure faster execution without compromising test coverage.
And CLI testing has two test scripts one is for export, compile, and execute, another is for infer cli api.
Note: This optimization is applied only to causal language models.