Skip to content

fix(logging): suppress faiss AVX2/AVX-512 fallback noise#1222

Merged
kovtcharov-amd merged 3 commits into
mainfrom
fix/faiss-avx-log-noise
May 29, 2026
Merged

fix(logging): suppress faiss AVX2/AVX-512 fallback noise#1222
kovtcharov-amd merged 3 commits into
mainfrom
fix/faiss-avx-log-noise

Conversation

@kovtcharov-amd
Copy link
Copy Markdown
Collaborator

Users on Linux (especially Arch/CachyOS via the npm install path) see 6 alarming-looking INFO lines from faiss.loader at startup — "Could not load library with AVX512 support", "Could not load library with AVX2 support" — and blame faiss for 50-second response times that are actually LLM inference on Gemma-4. The noise trains users to file the wrong bug.

Adds a filter_faiss_loader log filter (same pattern as the existing aiohttp/httpx/datasets/phonemizer suppressions in GaiaLogger) and replaces the 6 lines with a single summary in the server boot sequence that names the SWIG backend actually loaded.

Test plan

  • pytest tests/unit/test_faiss_log_filter.py -xvs — 10 tests covering every message variant (suppress attempts/failures, keep success/unrelated)
  • Start Agent UI (gaia chat --ui), confirm logs show one faiss: loaded (generic …) line instead of 6 noisy fallback messages
  • On a system with AVX2 faiss wheel, confirm log shows faiss: loaded with AVX2 support

@github-actions github-actions Bot added the tests Test changes label May 28, 2026
faiss-cpu's loader tries swigfaiss_avx512 → swigfaiss_avx2 → swigfaiss
and logs each failed attempt at INFO, producing 6 lines that look like
errors to users.  On PyPI wheels that ship only the generic SWIG module
(common on Linux), this made users think faiss was broken and was the
cause of slow responses — when the actual bottleneck is LLM inference.

Adds a log filter (same pattern as the existing aiohttp/httpx/datasets/
phonemizer filters) that suppresses the noisy fallback messages, and
replaces them with a single summary line in the server boot sequence
that names the optimization level actually loaded.
@kovtcharov-amd kovtcharov-amd force-pushed the fix/faiss-avx-log-noise branch from 6172ac2 to b9ee8ee Compare May 28, 2026 20:35
@kovtcharov-amd
Copy link
Copy Markdown
Collaborator Author

@claude review this PR

@github-actions
Copy link
Copy Markdown
Contributor

Clean, targeted DX fix that eliminates 6 alarming-looking startup lines and replaces them with a single informative summary. Follows the established GaiaLogger filter pattern exactly and ships thorough unit tests. One dead variable worth cleaning up; otherwise ready to merge.


Issues

🟢 Minor — dead variable filter_fns in test_filter_installed_on_logger (tests/unit/test_faiss_log_filter.py:168)

filter_fns is computed but never referenced — the assert any(...) below iterates faiss_logger.filters directly. Remove the unused assignment.

    def test_filter_installed_on_logger(self, gaia_logger):
        """GaiaLogger.__init__ installs the filter on the faiss.loader logger."""
        faiss_logger = logging.getLogger("faiss.loader")
        # The filter is installed as a bound method; check it's present by
        # verifying at least one filter on faiss.loader matches ours.
        assert any(

Strengths

  • Pattern consistency: filter_faiss_loader mirrors the four existing suppressors (filter_aiohttp_access, filter_datasets, filter_httpx, filter_phonemizer) in both registration style (logger.py:175) and filter shape — no new abstraction needed.
  • server.py is a genuine improvement: replacing 6 opaque fallback lines with a single faiss: loaded with AVX2 support (or the generic equivalent) makes startup logs actionable rather than alarming.
  • Test coverage is solid: 10 cases cover every suppressed variant plus the passthrough cases (Successfully loaded, unrelated messages), and the fixture correctly writes to a tmp_path to avoid side effects.

Verdict

Approve — one nit to clean up, no blocking issues.

@github-actions
Copy link
Copy Markdown
Contributor

Reviewed — this is clean and the core correctness concern (timing) holds up. Looks good to merge with a couple of minor notes below.

The thing that could have broken this is import ordering: faiss.loader emits those INFO lines exactly once, at the first import faiss in the process, and a log filter only suppresses records emitted after it's attached. That works here because log_manager = GaiaLogger() is a module-level singleton (src/gaia/logger.py:256), so the filter lands on the faiss.loader logger the instant gaia.logger is first imported — and every faiss import in the tree is deferred inside a function (src/gaia/rag/sdk.py:38, src/gaia/code_index/sdk.py, src/gaia/agents/base/memory.py, src/gaia/ui/server.py:249). Since gaia.logger is imported at process entry, the filter is always installed first. Good. The suppression also lives in logger.py (global), so it covers gaia chat CLI and code-index paths too, not just the UI server — the server.py summary is a nice extra on top.

The filter substrings ("Could not load", "Loading faiss with", "Loading faiss.") match faiss's real loader strings, and the startswith/in checks generalize cleanly to the AVX-512 SPR and SVE tiers as well. This is log-noise suppression, not a behavioral fallback, so no conflict with the no-silent-fallback rule — faiss still loads and a hard failure still raises. Tests are thorough and don't require faiss installed (they build LogRecords directly), so they'll run in CI fine.

🟢 Two minor things, neither blocking:

  1. The filter intentionally keeps "Successfully loaded faiss…", so the result is faiss's own success line plus the new faiss: loaded with … support summary — slightly redundant rather than the literal "one line" the description implies. Fine as-is; just flagging the wording.
  2. The test fixture instantiates a fresh GaiaLogger per test, and each __init__ re-adds the bound-method filter to the global faiss.loader logger, so filters accumulate across tests. Harmless (only the singleton runs in prod), but if you wanted it tidy you could guard against duplicate filters in __init__.

Nice fix — the "users blame faiss for Gemma-4 inference latency" framing is a real support-noise win.

@github-actions
Copy link
Copy Markdown
Contributor

Solid, minimal fix that follows the established GaiaLogger suppression pattern exactly. Two small nits worth addressing; nothing blocking.


Summary

The change correctly slots filter_faiss_loader into the same registration pattern used for aiohttp, httpx, datasets, and phonemizer — install a bound-method filter on a named logger in __init__. The server-side replacement (one informative line instead of six alarming ones) is the right UX call. Test coverage is thorough: 10 cases covering every suppress/keep variant, including a fixture-installation check.


Issues Found

🟢 Minor — Docstring exceeds CLAUDE.md's "one short line" rule (src/gaia/logger.py:203)

The four-line docstring on filter_faiss_loader is longer than CLAUDE.md allows for WHY comments ("keep WHY comments to one short line"). The docstring already restates the inline comment added above the addFilter call, so it's redundant.

    def filter_faiss_loader(self, record):
        # faiss INFO fallback messages look like errors; "Successfully loaded" is kept.
        msg = record.getMessage()

🟢 Minor — import sys belongs at module top, not inside the inner function (src/gaia/ui/server.py:247)

sys is a stdlib module that is always already in sys.modules; importing it inside _import_modules is unconventional and easy to miss. The # pylint: disable=unused-import comment above applies only to faiss/sentence_transformerssys is actively used, so the disable isn't needed for it. Moving sys to module-level (where it almost certainly already is elsewhere in the file) keeps the function's local-import block focused on the heavy third-party libs it's pre-warming.

            import faiss  # noqa: F401
            import sentence_transformers  # noqa: F401

and add import sys to the file's top-level imports (or rely on it already being there).


Strengths

  • Pattern consistency: filter_faiss_loader is structurally identical to filter_httpx, filter_datasets, and filter_phonemizer — same registration site in __init__, same return False / return True convention. A future reader doesn't need to learn a new idiom.
  • sys.modules check is correct: checking what faiss left in sys.modules after import is the right way to detect which SWIG backend won the race; it avoids re-importing or inspecting faiss internals.
  • Test quality: _make_record as a @staticmethod helper keeps every test a clean one-liner. The test_filter_installed_on_logger case catches a class of regression (filter registered on wrong logger name, or not registered at all) that unit tests on the method alone would miss.

Verdict

Approve with suggestions — the two nits above are cosmetic and don't affect correctness. Safe to merge as-is; the suggestions are optional but keep the file consistent with CLAUDE.md style.

@kovtcharov-amd kovtcharov-amd requested a review from itomek-amd May 29, 2026 01:10
@kovtcharov-amd kovtcharov-amd self-assigned this May 29, 2026
@kovtcharov-amd kovtcharov-amd enabled auto-merge May 29, 2026 02:57
@kovtcharov-amd kovtcharov-amd added this pull request to the merge queue May 29, 2026
Merged via the queue into main with commit fa64caf May 29, 2026
28 checks passed
@kovtcharov-amd kovtcharov-amd deleted the fix/faiss-avx-log-noise branch May 29, 2026 02:59
kovtcharov-amd added a commit that referenced this pull request May 29, 2026
Users on Linux (especially Arch/CachyOS via the npm install path) see 6
alarming-looking INFO lines from faiss.loader at startup — "Could not
load library with AVX512 support", "Could not load library with AVX2
support" — and blame faiss for 50-second response times that are
actually LLM inference on Gemma-4. The noise trains users to file the
wrong bug.

Adds a `filter_faiss_loader` log filter (same pattern as the existing
aiohttp/httpx/datasets/phonemizer suppressions in `GaiaLogger`) and
replaces the 6 lines with a single summary in the server boot sequence
that names the SWIG backend actually loaded.

## Test plan

- [ ] `pytest tests/unit/test_faiss_log_filter.py -xvs` — 10 tests
covering every message variant (suppress attempts/failures, keep
success/unrelated)
- [ ] Start Agent UI (`gaia chat --ui`), confirm logs show one `faiss:
loaded (generic …)` line instead of 6 noisy fallback messages
- [ ] On a system with AVX2 faiss wheel, confirm log shows `faiss:
loaded with AVX2 support`

---------

Co-authored-by: Ovtcharov <kovtchar@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants