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.

Sign up for GitHub

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

Jump to bottom

Support creating TestMo "resource" files #32

Merged
merged 22 commits into from
Oct 1, 2024
Merged

Support creating TestMo "resource" files #32

merged 22 commits into from
Oct 1, 2024
+95 −3

Conversation

derekk-nm
Copy link
Contributor

Summary:

add support for creating TestMo "resource" files and allow passing one to the testmo-run-submit-thread action.
This is to enable reporting resources when submitting LM-Eval run results to TestMo. Explicit testing of that will verify that we get the expected behavior when resource file is included.

Details:

  • adds a new testmo-create-resources action to create any resource file
  • adds a python script called by that action, compose_testmo_resources_file.py, that generates the proper JSON
  • updates testmo-run-submit-thread to generate a resource file with specified contents, and pass that in when submitting a new thread.
  • instructs ruff to ignore S603 issues, whose recommended solution contradicts S602. see Potential conflicted rules between S602 and S603 astral-sh/ruff#4045

Test Plan:

  • tested with the a branch of nm-vllm-ent where these are being used.

Related Issues:

n/a

@derekk-nm
support creation and use of testmo resource files
2698d99
@derekk-nm
pass testmo command as list of strings
41f9bbf
@derekk-nm
redefine testmo_args as list
f7c2176
@derekk-nm
still use shell=True
ce42b09
@derekk-nm
skip shell=True, but ignore ruff S603
8647e44
@derekk-nm
ruff style updates
340245c
@derekk-nm
Keep shell=True after all
fee3dad
@derekk-nm
and skip the ruff error
7206617
@derekk-nm
properly reference thread_resources
c3cf2dc
@derekk-nm
include some useful debugging output
ae8e709 
and don't capture_output from subprocess so that it ended up in stdout/stderr for debugging.
@derekk-nm
block quality check flag on debugging print statement
cd840cd
@derekk-nm
add more debugging statements
d8ad39f
@derekk-nm
minor fixes to debug
3742889
@derekk-nm
more debug statements
da80280
@derekk-nm
try quoting attribute values for add-field
c169df9
@derekk-nm
only double quotes
6a1b88d
@derekk-nm
set testmo token when calling add-field
5f66987
@derekk-nm
remove some debugging lines and minimize subprocess call
4b6da10
@derekk-nm
fix quality check fail
ef74d9f
@derekk-nm
fix quality check fail
58e5352
@derekk-nm
remove token input
ec2093f
@derekk-nm derekk-nm mentioned this pull request Sep 30, 2024
Closed
@derekk-nm
Copy link
Contributor Author

This PR replaces #24

dbarbuzzi
dbarbuzzi reviewed Sep 30, 2024
View reviewed changes
Copy link
Contributor

@dbarbuzzi dbarbuzzi left a comment

Choose a reason for hiding this comment

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

Looks good pending the removal of the change to ruff.toml.

ruff.toml Show resolved Hide resolved
andy-neuma
andy-neuma reviewed Sep 30, 2024
View reviewed changes
Copy link
Member

@andy-neuma andy-neuma left a comment

Choose a reason for hiding this comment

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

okay

actions/testmo-create-resources/compose_testmo_resources_file.py Show resolved Hide resolved
@derekk-nm derekk-nm added this pull request to the merge queue Oct 1, 2024
Merged via the queue into main with commit 909ee01 Oct 1, 2024
1 check passed
@derekk-nm derekk-nm deleted the dmk/testmo_resources_2 branch October 1, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy-neuma andy-neuma andy-neuma left review comments

@dbarbuzzi dbarbuzzi dbarbuzzi approved these changes

@dhuangnm dhuangnm Awaiting requested review from dhuangnm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@derekk-nm @dbarbuzzi @andy-neuma