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

[TLM CI] Add more configurable args to unittests #319

Merged
merged 2 commits into from
Sep 13, 2024
Merged

Conversation

huiwengoh
Copy link
Contributor

Add args to be able to specify which configs are not expected to produce trustworthiness scores or TLMResponse objects

Copy link

Ensure final changes made to the TLM code are tested before merging. To run the TLM tests, comment /test-tlm on this PR.

@huiwengoh
Copy link
Contributor Author

huiwengoh commented Sep 13, 2024

/test-tlm
Starting TLM tests...
If you want to run the TLM tests again (in case they failed or you have further updates to the TLM code), comment '/test-tlm' on this PR.
View full GitHub Actions run log
Tests completed!
TLM Tests Results: ✅✅ ✅✅✅
TLM Property Tests Results: ❌❌❌❌❌
Click the Github Actions run log for more information.
Tests completed!
TLM Tests Results: ✅✅ ✅✅✅
TLM Property Tests Results: ✅✅✅✅✅
Click the Github Actions run log for more information.
Tests completed!
TLM Tests Results: ❌❌ ❌❌❌
TLM Property Tests Results: ❌❌❌❌❌
Click the Github Actions run log for more information.
Tests completed!
TLM Tests Results: ❌❌ ❌❌❌
TLM Property Tests Results: ❌❌❌❌❌
Click the Github Actions run log for more information.

@huiwengoh huiwengoh changed the title [TLM] Add more configurable args to unittests [TLM CI] Add more configurable args to unittests Sep 13, 2024
@huiwengoh
Copy link
Contributor Author

huiwengoh commented Sep 13, 2024

@ulya-tkch fyi some of the the tests seem to be a little flaky, this first run failed, but when I re-ran it (without changing anything at all), everything passed this time.

My guess is that it might have something to do with the randomization of the args, but I am not sure (I was trying to debug locally but was unable to reproduce as well). Just flagging to keep an eye out in the future.

@huiwengoh
Copy link
Contributor Author

huiwengoh commented Sep 13, 2024

✨ Tests completed! ✨
TLM Tests Results: ❌❌ ❌❌❌
TLM Property Tests Results: ❌❌❌❌❌
Click the Github Actions run log for more information.

I am also unsure what happened here? Is it because I added a comment?

Edit: I think it is bc I added a comment, a second set of that has appeared haha

@ulya-tkch
Copy link
Collaborator

Yeah so the PR is merged to prevent that from happening, but the results were posted everytime a comment was added. This should no longer happen. Curent comments:

✨ Tests completed! ✨
TLM Tests Results: ✅✅ ✅✅✅
TLM Property Tests Results: ❌❌❌❌❌
Click the Github Actions run log for more information.
✨ Tests completed! ✨
TLM Tests Results: ✅✅ ✅✅✅
TLM Property Tests Results: ✅✅✅✅✅
Click the Github Actions run log for more information.
✨ Tests completed! ✨
TLM Tests Results: ❌❌ ❌❌❌
TLM Property Tests Results: ❌❌❌❌❌
Click the Github Actions run log for more information.
✨ Tests completed! ✨
TLM Tests Results: ❌❌ ❌❌❌
TLM Property Tests Results: ❌❌❌❌❌
Click the Github Actions run log for more information.

Copy link
Collaborator

@ulya-tkch ulya-tkch left a comment

Choose a reason for hiding this comment

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

[nit] Shouldn't _test_batch_get_trustworthiness_score_response(responses, options) also be updated for models with no perplexity score?

@huiwengoh
Copy link
Contributor Author

FYI, I am not changing the is_trustworthiness_score functions bc I think #304 is introducing much larger changes

@huiwengoh huiwengoh merged commit 4eef889 into main Sep 13, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants