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

enable HFDetector model configuration with hf_args #810

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

jmartin-tech
Copy link
Collaborator

@jmartin-tech jmartin-tech commented Jul 30, 2024

Fix #803

Allows hf_args configuration for simple HFDetectors:

  • default to cpu device when no configuration is provided for the detector
  • use AutoConfig to pass device to from_pretrained
  • ensure MustContradictNLI.detect() places prompt for evaluation in the configured device space
  • enable configuration of detector_model_path and detector_target_class

Example config of detector using cuda:
detector_cuda.yaml

plugins:
  detectors:
    misleading:
      MustContradictNLI:
        hf_args:
          device: cuda

The format above is based on HFDetector being a class specifically designed to use a TextClassificationPipeline. Is the is the approach desired or is further abstraction of ModelAsJudge be a goal of this revision?

Test example:

python -m garak -m nim -n meta/llama3-8b-instruct -p misleading.FalseAssertion50 --config detector_cuda.yaml

Logs and nvidia-smi show detector model loading in GPU.

More validation of compatible models may be desired if this pattern is determined to be a way forward.

@leondz
Copy link
Collaborator

leondz commented Jul 31, 2024

The format above is based on HFDetector being a class specifically designed to use a TextClassificationPipeline. Is the is the approach desired or is further abstraction of ModelAsJudge be a goal of this revision?

I anticipate ModelAsJudge primarily using external APIs, and so not using the HFDetector abstraction. It may later make sense to rename HFDetector to HFClassificationDetector to make its coupling to TextClassificationPipeline clear.

@leondz
Copy link
Collaborator

leondz commented Jul 31, 2024

Thanks for this, working great.

Q - both misleading.MustContradictNLI and misleading.MustRefuteClaimModel, that are associated with the misleading probes, can use cuda. Is it possible to configure these (or even all HFDetectors) in one go?

This ofc works:

plugins:
  detectors:
    misleading:
      MustContradictNLI:
        hf_args:
          device: cuda
      MustRefuteClaimModel:
        hf_args:
          device: cuda

Configuring the base class as below didn't work, I guess predicated on how config matching is applied, probably class name matching is more conservative/tidy than isinstance() testing:

plugins:
  detectors:
    base:
      HFDetector:
        hf_args:
          device: cuda

Is there a way to configure groups of plugins?

@leondz
Copy link
Collaborator

leondz commented Jul 31, 2024

Side issue - moving HFDetector to GPU exposes a further possible optimisation through using an HF dataset. This message comes up, presumably from transformers:

You seem to be using the pipelines sequentially on GPU. In order to maximize efficiency please use a dataset

The optimisation is out of scope here; tracking in #812

@jmartin-tech
Copy link
Collaborator Author

jmartin-tech commented Jul 31, 2024

Is there a way to configure groups of plugins?

Yes although it might be too broad in some detectors. We can configure at the module level. Currently there is no support for configuration of all implemented classes of a base plugin.

This would provide hf_args for all misleading detectors. Configurable intentionally

plugins:
  detectors:
    misleading:
      hf_args:
        device: cuda

While I like the idea of having less configuration required, I think in practical usage explicit configuration for a base class would be overkill and requires more internal knowledge of the implementation structure than would be typical for most users. I am skeptical of the trade off in value for powerusers vs complexity, maintenance and support for configuration by instance type.

@leondz
Copy link
Collaborator

leondz commented Jul 31, 2024

Yeah, if doing this to base classes requires extra code I'm having a hard time justifying that too. Thanks for the yaml, this works fine.

@leondz leondz merged commit 466ea05 into NVIDIA:main Aug 1, 2024
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2024
@jmartin-tech jmartin-tech deleted the feature/HFDetector-model-config branch August 14, 2024 19:39
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.

unused GPU in pipeline unless configured
2 participants