Skip to content

lazy load jailbreak detection dependencies #1223

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jeffreyscarpenter
Copy link

Description

Fixes #1222 by making jailbreak detection dependencies lazy-loaded. This means torch, transformers, and sklearn are only imported when using the local model-based approach, not when using the NIM-based approach.

Related Issue(s)

Fixes #1222

Checklist

  • I've read the CONTRIBUTING guidelines.
  • I've updated the documentation if applicable.
  • I've added tests if applicable.
  • @mentions of the person or team responsible for reviewing proposed changes.

Copy link
Collaborator

@cparisien cparisien left a comment

Choose a reason for hiding this comment

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

I'm ok with this. Will need @erickgalinkin to ensure current test coverage is sufficient.

Copy link
Collaborator

@erickgalinkin erickgalinkin left a comment

Choose a reason for hiding this comment

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

This looks great to me! Thanks for the suggestion @jeffreyscarpenter !

Copy link
Collaborator

@tgasser-nv tgasser-nv 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, could you add tests to ensure the correct modules are imported and the functions still work as before?

@@ -71,6 +73,7 @@ def __call__(self, text: str):

class JailbreakClassifier:
def __init__(self, random_forest_path: str):
from sklearn.ensemble import RandomForestClassifier
self.embed = SnowflakeEmbed()
with open(random_forest_path, "rb") as fd:
self.classifier = pickle.load(fd)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Should we include a pickle in the init() along with the RandomForestClassifier? My guess is scikit-learn imports it so we pick it up implicitly


@lru_cache()
def initialize_model(classifier_path: str = models_path) -> JailbreakClassifier:
def initialize_model(classifier_path: str = models_path):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Could you add the return -type back in (as 'JailbreakClassifier' now we're importing this in the function)?

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.

bug: Jailbreak detection imports unnecessary dependencies when using NIM
4 participants