data file override support with precedence #916

jmartin-tech · 2024-09-17T20:04:54Z

Provides a custom path accessor for project data files to allow user's to override project provided datasets using $XDG_DATA_DIR/garak/data to mirror garak/data from the package.
Separates non-code data or dataset files from source code
Downloaded datasets are considered cached files and do not change location or behavior in the revision
Removes argument parser no longer supported by gcg

Consumers of the garak.data.path object can traverse the data directory as a standard Path object and will be returned existing files based on the order precedence from the supported locations on disk.

A possible drawback to the current implementation exists regarding directory interaction, if a folder exists at more than one path location the higher precedence path will be returned, when performing glob or iteration actions with such an instance the only files in the selected path on disk will be evaluated. This is currently considered an acceptable limitation.

Verification

List the steps needed to make sure this thing works

Run the tests and ensure they pass python -m pytest tests/
garak -m nim -p donotanswer.MisinformationHarms -g 1 -n meta/llama3-70b-instruct --parallel_attempts 5
create a limited data set for donotanswer.MisinformationHarms

mkdir -p ~/.local/share/garak/data/donotanswer
head -n 10 garak/data/donotanswer/misinformation_harms.txt > ~/.local/share/garak/data/donotanswer/misinformation_harms.txt

garak -m nim -p donotanswer.MisinformationHarms -g 1 -n meta/llama3-70b-instruct --parallel_attempts 5
Verify run results:
- the first run reported 155 attempts
- the second run report 10 attempts (count depends -n count passed to head)

Note: #870 provides new data files that will need to migrate to this pattern depending on PR landing order.

Signed-off-by: Jeffrey Martin <[email protected]>

* returns first found instance of filename * custom `Path` raises exception when: * path escape attempt is detected * no file matching request exists

Signed-off-by: Jeffrey Martin <[email protected]>

* move to tools path * update to overwrite default `data` file Signed-off-by: Jeffrey Martin <[email protected]>

jmartin-tech · 2024-09-17T22:29:01Z

Looks like there is some work to do with support for this on python 3.10 apparently!

leondz · 2024-09-18T06:14:48Z

A possible drawback to the current implementation exists regarding directory interaction, if a folder exists at more than one path location the higher precedence path will be returned, when performing glob or iteration actions with such an instance the only files in the selected path on disk will be evaluated. This is currently considered an acceptable limitation.

Flagging a tension here with payload enumeration. Currently payload enum globs but uses its own directory traversal code, rather than garak.data. The result is that this PR doesn't break payload enum, but does leave us with two bits of code using similar-but-not-identical logic.

leondz

A few questions, looks good so far. Will be some extra work involved in aligning the payloads PR to the proposals here. The general separation is for non-code that can also be overridden should move into data; the rest remains in resources.

leondz · 2024-09-18T06:19:27Z

garak/data/__init__.py

+        _config.transient.data_dir / "data",
+        _config.transient.package_dir / "data",


realising now that these aren't particularly transient pieces of information, so might go elsewhere in config. one advantage of this PR is that if this is worth updating one day, the update can be centralised in data and made in far fewer places.

garak/data/__init__.py

garak/probes/topic.py

tests/test_data.py

tools/termscrape.py

Signed-off-by: Jeffrey Martin <[email protected]>

garak/data/__init__.py

erickgalinkin · 2024-09-18T13:27:55Z

garak/detectors/riskywords.py

 from garak.detectors.base import StringDetector

 surge_list = defaultdict(list)
 with open(
-    _config.transient.package_dir / "resources" / "profanity_en.csv",
+    data_path / "profanity_en.csv",


Probably worth looking at as part of a larger refactor -- do we want all these datasets in our git? Or should we have them in some other place e.g. HuggingFace hub, and the garak.data module can also manage downloading these files?

Have discussed in the past that we may want tooling that will download all datasets to create an offline deployment capability.

I suspect there could be some expansion on garak.data for handling access to known datasets. Treating HF as the specific location that tooling can register data as available from might be a good direction.

Sometimes HF drops connections and things go wrong, so I prefer keeping smaller things closer. What that cutoff is, I don'k know - and it does also mean that garak will grow bigger over time.

Agree some expansion could work, perhaps using HF by default with a backup URI also (maybe a garak-data repo)

garak/resources/common.py

erickgalinkin · 2024-09-18T13:42:42Z

garak/resources/tap/tap_main.py

-    / "tap"
-    / "data"
-    / "tap_jailbreaks.txt"
+    garak._config.transient.cache_dir / "data" / "tap" / "tap_jailbreaks.txt"


Since probes.tap references the prompts_location as data_path / "tap" / "tap_jailbreaks.txt", do we want this to reflect that as well? The idea is to keep your TAP results such that you can use them (or share them with us!) in subsequent probes. Though perhaps that's a larger issue.

I agree these should be captured better for re-use. I will defer that change to a future iteration though. I think there is a larger issue to address as we have a few techniques that generate these intermediate files that could be valuable from a research context.

garak/probes/visual_jailbreak.py

garak/resources/common.py

jmartin-tech · 2024-09-18T14:27:03Z

Flagging a tension here with payload enumeration. Currently payload enum globs but uses its own directory traversal code, rather than garak.data. The result is that this PR doesn't break payload enum, but does leave us with two bits of code using similar-but-not-identical logic.

Depending on land order I am thinking this can be a use case to improve glob support on the custom Path objects to return a consolidated list of files based on searching all ORDERED_SEARCH_PATHS. This would then remove the similar-but-not-identical logic by supporting the need quickly as a next iteration.

Signed-off-by: Jeffrey Martin <[email protected]>

jmartin-tech · 2024-09-18T16:24:43Z

garak/probes/topic.py

@@ -92,7 +92,8 @@ def _gen_prompts(self, term):
    def __init__(self, config_root=_config):
        super().__init__(config_root)

-        self.data_dir = _config.transient.cache_dir / "resources" / "wn"
+        self.data_dir = _config.transient.cache_dir / "data" / "wn"
+        self.data_dir.parent.mkdir(mode=0o740, parents=True, exist_ok=True)


Interesting item here, the parent must exist before download however if the wn folder already exists the raise when attempting access below will be a different exception type and fails to recover.

This was masked previously by the fact that _config.transient.cache_dir / "resources" is created in early initialization for the plugin_cache.json file.

Oh, interesting. Where do we want to check for / make XDG dirs? And then I guess should we be making subdirs using mkdir -p-equiv?

Where do we want to check for / make XDG dirs?
https://github.com/leondz/garak/blob/0e1d96e3892e14564aaece56c1ee9f92cfdd7dc0/garak/_config.py#L68-L71

parents=True is the equivalent to -p.

Thanks, missed that. This looks good. Expect to move wn code to its own module/class once we know where those will live - did we settle on resources? Other options could be apis, external, apps, common, & not lib. Or is this a conversation to conclude once this PR lands?

Signed-off-by: Jeffrey Martin <[email protected]>

garak/payloads.py

Signed-off-by: Jeffrey Martin <[email protected]>

leondz

lgtm

Signed-off-by: Jeffrey Martin <[email protected]>

leondz · 2024-09-25T07:43:23Z

NICE

jmartin-tech added 5 commits September 16, 2024 11:39

remove parser no longer used for gcg

aa48895

Signed-off-by: Jeffrey Martin <[email protected]>

move rest demo to tools

724a81e

Signed-off-by: Jeffrey Martin <[email protected]>

add custom Path type for data files

8890c94

* returns first found instance of filename * custom `Path` raises exception when: * path escape attempt is detected * no file matching request exists

use garak.data.path to access data files

4a73965

Signed-off-by: Jeffrey Martin <[email protected]>

move slurprompt termscraper

bd773dc

* move to tools path * update to overwrite default `data` file Signed-off-by: Jeffrey Martin <[email protected]>

jmartin-tech requested review from leondz and erickgalinkin September 17, 2024 20:04

jmartin-tech marked this pull request as draft September 17, 2024 22:27

leondz reviewed Sep 18, 2024

View reviewed changes

leondz added the architecture Architectural upgrades label Sep 18, 2024

support for python 3.10+ Path objects

3405489

Signed-off-by: Jeffrey Martin <[email protected]>

erickgalinkin reviewed Sep 18, 2024

View reviewed changes

jmartin-tech commented Sep 18, 2024

View reviewed changes

garak/probes/visual_jailbreak.py Show resolved Hide resolved

jmartin-tech commented Sep 18, 2024

View reviewed changes

garak/resources/common.py Outdated Show resolved Hide resolved

ensure cache data path exists for download

47a2f3b

Signed-off-by: Jeffrey Martin <[email protected]>

jmartin-tech commented Sep 18, 2024

View reviewed changes

jmartin-tech marked this pull request as ready for review September 18, 2024 16:33

jmartin-tech added 5 commits September 18, 2024 14:09

Merge branch 'main' into feature/resource-precedence

cb54c27

get payloads from garak.data.path

184b5b3

Signed-off-by: Jeffrey Martin <[email protected]>

defer to garak.data for all payload file selection

2513ed9

Signed-off-by: Jeffrey Martin <[email protected]>

nltk_data fallback path in data dir

a10f820

Signed-off-by: Jeffrey Martin <[email protected]>

initialize advbench_base_path as local variable

f03d8d6

Signed-off-by: Jeffrey Martin <[email protected]>

jmartin-tech commented Sep 23, 2024

View reviewed changes

garak/payloads.py Show resolved Hide resolved

adjust expected error when payload file is not found

958ea3c

Signed-off-by: Jeffrey Martin <[email protected]>

leondz approved these changes Sep 24, 2024

View reviewed changes

clarify log for advbench source

93e8d62

Signed-off-by: Jeffrey Martin <[email protected]>

jmartin-tech merged commit 29c5b9a into NVIDIA:main Sep 24, 2024
8 checks passed

jmartin-tech deleted the feature/resource-precedence branch September 24, 2024 16:47

github-actions bot locked and limited conversation to collaborators Sep 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

data file override support with precedence #916

data file override support with precedence #916

jmartin-tech commented Sep 17, 2024

jmartin-tech commented Sep 17, 2024

leondz commented Sep 18, 2024

leondz left a comment

leondz Sep 18, 2024

erickgalinkin Sep 18, 2024

jmartin-tech Sep 18, 2024

leondz Sep 20, 2024

erickgalinkin Sep 18, 2024

jmartin-tech Sep 23, 2024

jmartin-tech commented Sep 18, 2024

jmartin-tech Sep 18, 2024 •

edited

Loading

leondz Sep 20, 2024

jmartin-tech Sep 23, 2024

leondz Sep 24, 2024

leondz left a comment

leondz commented Sep 25, 2024

		_config.transient.data_dir / "data",
		_config.transient.package_dir / "data",

data file override support with precedence #916

data file override support with precedence #916

Conversation

jmartin-tech commented Sep 17, 2024

Verification

jmartin-tech commented Sep 17, 2024

leondz commented Sep 18, 2024

leondz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmartin-tech commented Sep 18, 2024

jmartin-tech Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leondz left a comment

Choose a reason for hiding this comment

leondz commented Sep 25, 2024

jmartin-tech Sep 18, 2024 •

edited

Loading