refactor(providers): remove support for deprecated nemollm engine #1076

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.

Sign up for GitHub

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

Jump to bottom

Merged

Pouyanpi merged 4 commits into develop from refactor/deprecate-nemollm

Apr 14, 2025

Collaborator

Pouyanpi commented Mar 31, 2025 •

edited

Loading

Description

NeMo Service reached its End-of-Life on 5 Feb 2025

Documentation changes #1077

Pouyanpi self-assigned this

Pouyanpi added this to the v0.14.0 milestone

Pouyanpi commented

View reviewed changes

docs/user-guides/configuration-guide.md Outdated Show resolved Hide resolved

Pouyanpi commented

View reviewed changes

docs/user-guides/configuration-guide.md Outdated Show resolved Hide resolved

Pouyanpi commented

View reviewed changes

docs/user-guides/configuration-guide.md Outdated Show resolved Hide resolved

Pouyanpi commented

View reviewed changes

docs/user-guides/llm-support.md Show resolved Hide resolved

Pouyanpi commented

View reviewed changes

nemoguardrails/llm/filters.py

Collaborator Author

Pouyanpi Mar 31, 2025

will remove these functions in 0.15.0

Pouyanpi commented

View reviewed changes

nemoguardrails/llm/providers/nemollm.py Outdated Show resolved Hide resolved

Pouyanpi commented

View reviewed changes

nemoguardrails/llm/providers/providers.py Outdated Show resolved Hide resolved

Pouyanpi commented

View reviewed changes

nemoguardrails/llm/types.py

Collaborator Author

Pouyanpi Mar 31, 2025

FWIW, this is already fixed in #1040

Pouyanpi commented

View reviewed changes

tests/llm_providers/test_nemollm.py Outdated Show resolved Hide resolved

Pouyanpi commented

View reviewed changes

tests/test_bug_5.py

Collaborator Author

Pouyanpi Mar 31, 2025

what this test is trying to do 🫤

There will be another PR that restructure all the tests.

Pouyanpi commented

View reviewed changes

tests/test_fact_checking.py

Collaborator Author

Pouyanpi Mar 31, 2025

This is already fixed in #1040

Pouyanpi commented

View reviewed changes

Collaborator Author

Pouyanpi left a comment •

edited

Loading

remove nemoguardrails/llm/providers/nemollm.py
remove tests/llm_providers/test_nemollm.py
fix table in docs/user-guides/llm-support.md -> https://nvidia.github.io/NeMo-Guardrails/review/pr-1076/user-guides/llm-support.html#llm-support-and-guidance
raise exception instead of warning nemoguardrails/llm/providers/providers.py ->

Pouyanpi requested review from mikemckiernan and tgasser-nv

March 31, 2025 13:01

NVIDIA-NeMo deleted a comment from github-actions bot

Contributor

github-actions bot commented Mar 31, 2025

Documentation preview

https://nvidia.github.io/NeMo-Guardrails/review/pr-1076

Pouyanpi marked this pull request as ready for review

March 31, 2025 13:28

Pouyanpi force-pushed the refactor/deprecate-nemollm branch 2 times, most recently from 17bf3f3 to 86a7498 Compare

March 31, 2025 15:20

Pouyanpi mentioned this pull request

docs: remove NeMo Service (nemollm) related documentation #1077

Merged

Pouyanpi force-pushed the refactor/deprecate-nemollm branch from 86a7498 to 3dcff99 Compare

March 31, 2025 15:22

Pouyanpi added the status: in review label

Pouyanpi changed the title ~~refactor: remove support for deprecated nemollm engine~~ refactor(providers): remove support for deprecated nemollm engine

Pouyanpi added the refactoring label

tgasser-nv previously approved these changes

View reviewed changes

Collaborator

tgasser-nv left a comment

LGTM, approving! Just one place with nemollm referenced:

./tests/test_configs/autoalign/config.yml:3: engine: nemollm

Could you swap this out for a Llama or openai model and then merge?

trebedea previously approved these changes

View reviewed changes

tests/test_configs/fact_checking/config.yml Outdated Show resolved Hide resolved

trebedea reviewed

View reviewed changes

Member

trebedea left a comment

@Pouyanpi , some further comments.

tests/test_fact_checking.py Outdated Show resolved Hide resolved

nemoguardrails/llm/filters.py Outdated Show resolved Hide resolved

nemoguardrails/llm/taskmanager.py

    
                      self.env.filters["last_turns"] = last_turns

                      self.env.filters["indent"] = indent

                      self.env.filters["user_assistant_sequence"] = user_assistant_sequence

                      self.env.filters[

Member

trebedea Apr 11, 2025

If user_assistant_sequence_nemollm and to_messages_nemollm are removed from here, does it make sense to keep them in filters.py ?

Collaborator Author

Pouyanpi Apr 11, 2025

removed 👍🏻

Pouyanpi force-pushed the refactor/deprecate-nemollm branch from c5cd3df to 3aa2d0a Compare

April 11, 2025 10:03

Collaborator Author

Pouyanpi commented Apr 11, 2025

LGTM, approving! Just one place with nemollm referenced:

./tests/test_configs/autoalign/config.yml:3: engine: nemollm

Could you swap this out for a Llama or openai model and then merge?

Thank you @tgasser-nv , just removed it 👍🏻

Pouyanpi dismissed stale reviews from trebedea and tgasser-nv via

7ccbd62

April 11, 2025 10:26

Pouyanpi requested a review from trebedea

April 11, 2025 10:26

trebedea previously approved these changes

View reviewed changes

Pouyanpi force-pushed the refactor/deprecate-nemollm branch from 7ccbd62 to 3835723 Compare

April 11, 2025 15:22

Collaborator Author

Pouyanpi commented Apr 11, 2025

now I have not signed my commits!

Pouyanpi added 4 commits

April 14, 2025 12:09


          refactor: remove support for deprecated NeMo LLM model

0b5ccad


          fix: apply review suggestions

fa3e677


          remove deprecated functions from filters.py

d5f02a6


          use nim engine to fix failing test

6fd0817

Pouyanpi dismissed trebedea’s stale review via

6fd0817

April 14, 2025 10:09

Pouyanpi force-pushed the refactor/deprecate-nemollm branch from 3835723 to 6fd0817 Compare

April 14, 2025 10:09

Pouyanpi requested a review from tgasser-nv

April 14, 2025 10:11

trebedea approved these changes

View reviewed changes

Pouyanpi merged commit 0a6968b into develop

27 checks passed

Pouyanpi deleted the refactor/deprecate-nemollm branch

April 14, 2025 10:41

Pouyanpi removed the status: in review label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels