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

Adding Tests for GGUFWriter Class #4311

Closed
wants to merge 0 commits into from

Conversation

mendax0110
Copy link
Contributor

Description

This pull request adds comprehensive tests for the GGUFWriter class. Previously, there were no tests available for this class.

Changes Made

  • Introduced tests for the following methods:
    • test_add_uint8
    • test_add_int8
    • test_add_uint16
    • test_write_tensors_to_file
    • test_add_description
    • test_add_uint32
    • test_add_float32

Purpose

The purpose of these tests is to ensure the reliability, correctness, and expected behavior of the GGUFWriter methods under different scenarios.

Benefits

  • Enhances code quality by ensuring existing functionalities work as intended.
  • Guards against potential regressions by establishing a testing suite for future changes.

Additional Notes

  • All tests have been successfully validated and pass without issues.
  • The tests leverage temporary directories and context managers to maintain a clean testing environment.

Copy link
Collaborator

@cebtenzzre cebtenzzre left a comment

Choose a reason for hiding this comment

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

You'll have to reduce the amount of copy-pasting by using one or more shared functions for this to be mergeable. You could also consider testing more than one feature at a time, which provides less fine-grained failure information but has the same level of coverage.

@mendax0110 mendax0110 requested a review from cebtenzzre December 5, 2023 22:33
@cebtenzzre
Copy link
Collaborator

The tests don't work. There's a warning:

E/usr/lib/python3.11/unittest/case.py:622: ResourceWarning: unclosed file <_io.BufferedWriter name='/tmp/tmpuro9bqgy/test4.gguf'>
  with outcome.testPartExecutor(self):
ResourceWarning: Enable tracemalloc to get the object allocation traceback

And an error:

======================================================================
ERROR: test_add_description (__main__.TestGGUFWriter.test_add_description)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jared/src/forks/llama.cpp/gguf-py/tests/test_gguf.py", line 58, in test_add_description
    gguf_writer.write_kv_data_to_file()
  File "/home/jared/src/forks/llama.cpp/gguf-py/gguf/gguf_writer.py", line 87, in write_kv_data_to_file
    raise ValueError(f'Expected output file to contain the header, got {self.state}')
ValueError: Expected output file to contain the header, got WriterState.EMPTY

For several tests.

@mendax0110
Copy link
Contributor Author

The tests don't work. There's a warning:

E/usr/lib/python3.11/unittest/case.py:622: ResourceWarning: unclosed file <_io.BufferedWriter name='/tmp/tmpuro9bqgy/test4.gguf'>
  with outcome.testPartExecutor(self):
ResourceWarning: Enable tracemalloc to get the object allocation traceback

And an error:

======================================================================
ERROR: test_add_description (__main__.TestGGUFWriter.test_add_description)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jared/src/forks/llama.cpp/gguf-py/tests/test_gguf.py", line 58, in test_add_description
    gguf_writer.write_kv_data_to_file()
  File "/home/jared/src/forks/llama.cpp/gguf-py/gguf/gguf_writer.py", line 87, in write_kv_data_to_file
    raise ValueError(f'Expected output file to contain the header, got {self.state}')
ValueError: Expected output file to contain the header, got WriterState.EMPTY

For several tests.

Bild 06 12 23 um 21 46

I cannot reproduce the errors you got. Can you give me the steps to reproduce these errors?

@cebtenzzre
Copy link
Collaborator

You must call write_header_to_file before write_kv_data_to_file. I know because I wrote those exceptions. The only reason I can think it would pass is if you are testing an old version of the gguf module - the latest is 0.6.0.

gguf-py/tests/test_gguf.py Outdated Show resolved Hide resolved
@cebtenzzre
Copy link
Collaborator

cebtenzzre commented Dec 12, 2023

The tests in this PR still don't work, please run pip install -U gguf to test with the latest gguf package. (I just published the latest one.)

@mendax0110
Copy link
Contributor Author

I did this. I will uninstall/install gguf again and try it again.

@github-actions github-actions bot added the python python script changes label May 26, 2024
@mendax0110 mendax0110 closed this Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python python script changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants