-
Notifications
You must be signed in to change notification settings - Fork 512
add integration tests for all problem types #380
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
Conversation
| import os | ||
|
|
||
| from app_utils.sections.chat import show_chat_is_running_dialog | ||
| from llm_studio.app_utils.sections.chat import show_chat_is_running_dialog |
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.
Like the idea to move app_utils to llm_studio! Is this required for the tests?
Also, in a recent discussion with @fatihozturkh2o, we where thinking of removing the utils part from app_utils folder or renaming it to something like frontend. WDYT?
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.
it's required for getting the test coverage right. I would also add this to the command. Currently looking into best ways for the report.
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.
yeah, this is what we planned pretty much:
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.
we also need to move the train.py and train_wave.py to the code subfolder for discoverability in pytest-coverage.
I'll push this and add symlinks probably to keep the old behavior in a later PR
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.
moving train.py and the other python files in the root dir is a bit more tricky than expected. I'd probably postpone for later, as it really only affects the coverage report.
add symbolic links rename datasets due to same pip package name
This reverts commit ccfbbe2.
1c6b778 to
90148e2
Compare
psinger
left a comment
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.
Thanks - great stuff!
The imports in hugging_face_utils.py are broken.
run a small experiment for each problem type via CLI
in github actions (no cuda available), the test will be skipped
locally, it will run a single GPU experiment that takes about 1 minute each
closes #333