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

fix: correct argument in TAP #1022

Merged
merged 1 commit into from
Dec 2, 2024
Merged

fix: correct argument in TAP #1022

merged 1 commit into from
Dec 2, 2024

Conversation

harshraj172
Copy link
Contributor

@harshraj172 harshraj172 commented Nov 24, 2024

Fix incorrect configuration variable in garak/resources/tap/tap_main.py

Fix

This PR addresses a bug in garak/resources/tap/tap_main.py where the incorrect variable name was used for the evaluator model configuration. Specifically, the line:

evaluator_model_configs=evaluator_model_config

was corrected to:

evaluator_model_config=evaluator_model_config

This fix ensures that the proper model configuration is being used when initializing the evaluator.

Copy link
Contributor

github-actions bot commented Nov 24, 2024

DCO Assistant Lite bot All contributors have signed the DCO ✍️ ✅

@leondz
Copy link
Collaborator

leondz commented Nov 24, 2024

Thanks! Will take a look

Is there a test that could have caught this?

@leondz
Copy link
Collaborator

leondz commented Nov 25, 2024

@harshraj172 could you sign the DCO please

@harshraj172
Copy link
Contributor Author

Thanks! Will take a look

Is there a test that could have caught this?

Sorry for the late reply. Do you want me to write a test which can catch this?

@harshraj172
Copy link
Contributor Author

@harshraj172 could you sign the DCO please
Yes, I will by EoD. thanks

@jmartin-tech jmartin-tech changed the title remove invalid argument in TAP fix: correct argument in TAP Nov 25, 2024
Copy link
Collaborator

@jmartin-tech jmartin-tech left a comment

Choose a reason for hiding this comment

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

The class impacted here does not currently have any tests in place, if coverage were added it could have caught this.

@harshraj172, we can accept this without an added unit test however you will need to complete the DCO signing process for us to be able to credit you for the change. Please add the comment requested by the bot and the trigger recheck comment to complete the signature.

@harshraj172
Copy link
Contributor Author

I have read the DCO Document and I hereby sign the DCO

I have read the DCO Document and I hereby sign the DCO

@harshraj172
Copy link
Contributor Author

recheck

@harshraj172
Copy link
Contributor Author

I have read the DCO Document and I hereby sign the DCO

@harshraj172
Copy link
Contributor Author

recheck

github-actions bot added a commit that referenced this pull request Nov 28, 2024
@leondz leondz merged commit 5495536 into NVIDIA:main Dec 2, 2024
9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants