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

probe: add Arabic donotanswer #1017

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

probe: add Arabic donotanswer #1017

wants to merge 3 commits into from

Conversation

Eaalghamdi
Copy link

Signed-off-by: Emad Alghamdi [email protected]

I added Arabic translation of all donotanswer probes which were quality checked by human to ensure suitability to the Arabic language, added Arabic detector for the probe. The new probes and detectors passed the test during development.

Signed-off-by: Emad Alghamdi <[email protected]>
Copy link
Contributor

github-actions bot commented Nov 21, 2024

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

@Eaalghamdi Eaalghamdi changed the title probe: add Arabic DNA probe: add Arabic donotanswer Nov 21, 2024
@Eaalghamdi
Copy link
Author

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

@Eaalghamdi
Copy link
Author

recheck

github-actions bot added a commit that referenced this pull request Nov 23, 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.

Thank you for this! Additional language support is currently evolving and this give some insight into how others are thinking about it.

I made a couple comments mostly related to location of code, long term form for this may change further, however this is great step towards enabling testing on targets in more languages.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think language specific detectors are likely better contained in the same module. In this case adding this class to garak/detectors/mitigation.py as it's own class is a preferred approach to make this available in the near term possibly as garak.detectors.mitigation.MitigationBypassArabic or MitigationBypassAR to use the bcp47 value as an indicator of language.

In a future iteration I would like to see garak.detectors.mitigation.MitigationBypass load the language specific values needed from a data file based on the language compatible with the probe that generated the attempt.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Language specific implementations are likely better contained in the same module. In this case adding these classes to garak/probes/donotanswer.py as it's own set of classes, based on a prefix or suffix for the language, is a preferred approach to make this available in the near term.

In a future iteration I would like to see garak.probes.donotanswer.* load the language specific values needed from a data file based on the language the run requires to be compatible with the target application.

Copy link
Author

Choose a reason for hiding this comment

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

I modified the original code to support Arabic (and potentially other languages). Now language specific implementation is included within the targeted probe and detector module. Finally, Arabic data path is changed to /data/ar/*

Copy link
Collaborator

Choose a reason for hiding this comment

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

A minor file structure item, consider using bcp47 values as a standard already adopted by the project to determine the storage path for language specific values, in this case garak/data/ar/* might provide a more programatic access friendly pattern locating files for a specific language.

Copy link
Author

Choose a reason for hiding this comment

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

fixed

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