Skip to content

Cache Matrix0 parameter count for health endpoint#104

Merged
lukifer23 merged 1 commit intomasterfrom
codex/reuse-cached-model-in-health-check
Oct 12, 2025
Merged

Cache Matrix0 parameter count for health endpoint#104
lukifer23 merged 1 commit intomasterfrom
codex/reuse-cached-model-in-health-check

Conversation

@lukifer23
Copy link
Owner

Summary

  • cache the Matrix0 model parameter count and reuse any loaded model in the web UI server
  • update the /health endpoint to return the cached value without instantiating PolicyValueNet repeatedly
  • extend the web UI server tests to assert that the /health endpoint does not build the model more than once

Testing

  • pytest tests/test_webui_server.py

https://chatgpt.com/codex/tasks/task_e_68e6f992a86083238c7118dc33ca7287

Copilot AI review requested due to automatic review settings October 12, 2025 21:56
@lukifer23 lukifer23 merged commit d5043e4 into master Oct 12, 2025
1 check failed
@lukifer23 lukifer23 deleted the codex/reuse-cached-model-in-health-check branch October 12, 2025 21:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the performance of the /health endpoint by caching the Matrix0 model parameter count instead of recreating the model on each request.

  • Introduces caching mechanism for Matrix0 model parameter count
  • Refactors /health endpoint to use cached values instead of instantiating PolicyValueNet repeatedly
  • Adds comprehensive test coverage to verify the caching behavior works correctly

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
webui/server.py Adds parameter count caching logic and refactors health endpoint to use cached values
tests/test_webui_server.py Adds test to verify health endpoint caches model parameter count and doesn't rebuild model unnecessarily

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

def _get_matrix0_param_count() -> Optional[int]:
"""Return the cached parameter count, computing it lazily if needed."""

global _matrix0_model_params, _cfg
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

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

Missing _matrix0_model in the global declaration. The function accesses _matrix0_model on line 785 but doesn't declare it as global, which could lead to UnboundLocalError if the global variable doesn't exist.

Copilot uses AI. Check for mistakes.
Comment on lines +789 to +790
except Exception as exc: # pragma: no cover - defensive fallback
logger.debug("Failed to count parameters on loaded model: %s", exc)
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

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

Using broad Exception catching is discouraged. Consider catching more specific exceptions that count_parameters() might raise, or document why broad exception handling is necessary here.

Copilot uses AI. Check for mistakes.
finally:
del model
return _matrix0_model_params
except Exception as exc: # pragma: no cover - defensive fallback
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

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

Using broad Exception catching is discouraged. Consider catching more specific exceptions related to model configuration loading and parameter counting, or document why broad exception handling is necessary here.

Suggested change
except Exception as exc: # pragma: no cover - defensive fallback
except (FileNotFoundError, OSError, ValueError, AttributeError) as exc: # pragma: no cover - defensive fallback

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants