feat: granite4.1#964
Conversation
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
nrfulton
left a comment
There was a problem hiding this comment.
We should probably clean up the docs and code-base if we have specific model ids showing up this often.
| IBM_GRANITE_4_1_3B = ModelIdentifier( | ||
| hf_model_name="ibm-granite/granite-4.1-3b", | ||
| ollama_name="granite4.1:3b", | ||
| watsonx_name=None, |
| IBM_GRANITE_4_1_3B = ModelIdentifier( | ||
| hf_model_name="ibm-granite/granite-4.1-3b", | ||
| ollama_name="granite4.1:3b", | ||
| watsonx_name=None, |
Not sure I quite follow. Are we talking about instances of Granite4.0 in docs? |
ajbozarth
left a comment
There was a problem hiding this comment.
A few things to fix before this merges.
Commit hygiene (blocking)
Commits are missing Signed-off-by (git commit -s, AGENTS.md §6) and Assisted-by: Claude Code (AGENTS.md §7). The PR will be squash-merged, so at least one commit needs both trailers before then.
Tests (non-blocking)
No new unit tests for the four new ModelIdentifier constants — checklist box is unchecked.
| ## Hello world | ||
|
|
||
| By default, `start_session()` connects to Ollama and uses **IBM Granite 4 Micro** | ||
| (`granite4:micro`). Make sure Ollama is running before you run this: |
There was a problem hiding this comment.
Branding mismatch. granite4.1:3b is Granite 4.1, not "Granite 4 Micro" — that name belonged to 4.0-micro. This line and line 85 should be updated, e.g. to IBM Granite 4.1 3B.
|
|
||
| `start_session()` defaults to **Ollama** with **IBM Granite 4 Micro** (`granite4:micro`). | ||
| `start_session()` defaults to **Ollama** with **IBM Granite 4 Micro** (`granite4.1:3b`). | ||
| No API keys needed — just have Ollama running: |
There was a problem hiding this comment.
Same branding mismatch: points to granite4.1:3b but still says "IBM Granite 4 Micro".
| `start_session()` connects to Ollama on `localhost:11434` and uses | ||
| **IBM Granite 4 Micro** (`granite4:micro`) by default. On first run, Mellea | ||
| **IBM Granite 4 Micro** (`granite4.1:3b`) by default. On first run, Mellea | ||
| automatically pulls the model if it is not already downloaded: |
|
|
||
| **Default backend:** `start_session()` with no arguments connects to a local | ||
| [Ollama](https://ollama.ai) instance running **IBM Granite 4 Micro** | ||
| (`granite4:micro`). Make sure Ollama is running before you execute any example. |
|
|
||
| IBM_GRANITE_GUARDIAN_4_1_8B = ModelIdentifier( | ||
| hf_model_name="ibm-granite/granite-guardian-4.1-8b" | ||
| ) |
There was a problem hiding this comment.
The "removing Nones" fixup dropped ollama_name=None, # TBD from IBM_GRANITE_GUARDIAN_4_1_8B entirely — there's no longer any indication that Ollama support is pending vs. not applicable. Also, all four new constants now use a compact single-line style inconsistent with every other constant in the file. Suggest reverting to multi-line with ollama_name=None, # not yet available for the guardian.
There was a problem hiding this comment.
there's no longer any indication that Ollama support is pending vs. not applicable.
@avinash2692 Granite 4.1 is on ollama: https://ollama.com/library/granite4.1
| ollama pull granite4:micro | ||
| ollama pull granite4:micro-h | ||
| ollama pull granite4.1:3b | ||
| - name: Run Tests |
There was a problem hiding this comment.
granite4:micro-h was dropped from the pull list. Confirm no ollama-marked integration tests still use IBM_GRANITE_4_HYBRID_MICRO, or they'll cold-start/fail in CI.
There was a problem hiding this comment.
Yup, I think I confirmed that in a nightly run.
| ) | ||
| for model in ["granite4:micro", "granite4:micro-h", "granite3.2-vision"]: | ||
| for model in ["granite4.1:3b", "granite3.2-vision"]: | ||
| try: |
There was a problem hiding this comment.
Same concern: granite4:micro-h removed from the warm-up and eviction loops. Please verify no ollama-marked tests depend on the hybrid model being warm.
|
I believe all the items Claude found are either docs that could be handled in a follow up or non-issues, but it's worth double checking |
|
waiting on #970 to be resolved. |
Yea, I think there are some branding/docs issues there. Like @nrfulton mentioned (and if I understood what he said there) I think we might have to clean up model names in docs quite a bit. |
* updating core to granite4.1 * updating core to granite4.1 * updating tests to granite4.1 * updating ci to granite4.1 * adding 4.1 to all the docs * missed one * missed some more * removing Nones * adding xfail to a flaky test * setting watsonx name to none to ignore it in tests * temp hack for watsonx backend * adding warning to docstring
Misc PR
Type of PR
Description
Summary
IBM_GRANITE_4_1_3B,IBM_GRANITE_4_1_8B,IBM_GRANITE_4_1_30B, andIBM_GRANITE_GUARDIAN_4_1_8Bmodel identifiers to the registrygranite-4.0-micro(3B) togranite-4.1-3bgranite4:micro/granite4:micro-htogranite4.1:3bWhat changed
mellea/backends/model_ids.pyModelIdentifierconstants for 4.1mellea/stdlib/session.pyIBM_GRANITE_4_1_3Bmellea/backends/ollama.pyIBM_GRANITE_4_1_3Bmellea/backends/litellm.pyIBM_GRANITE_4_1_3Bcli/eval/runner.pyIBM_GRANITE_4_1_3B.github/workflows/quality.ymlollama pull granite4.1:3btest/conftest.pygranite4.1:3btest/scripts/granite4:micro(-h)→granite4.1:3bWhat was NOT changed (intentional)
granite-4.0-microIBM_GRANITE_4_MICRO_3Bconstant — kept for backward compatibilityIBM_GRANITE_4_HYBRID_MICRO— hybrid model unaffected, Ollama tag unchangedTesting
Attribution