Skip to content

add LFM example#1504

Merged
charlesfrye merged 8 commits intomainfrom
charlesfrye/liquidai-lfm
Feb 21, 2026
Merged

add LFM example#1504
charlesfrye merged 8 commits intomainfrom
charlesfrye/liquidai-lfm

Conversation

@charlesfrye
Copy link
Copy Markdown
Collaborator

@charlesfrye charlesfrye commented Feb 21, 2026

@charlesfrye charlesfrye requested a review from shababo February 21, 2026 02:32
@charlesfrye
Copy link
Copy Markdown
Collaborator Author

@prbot approve @shababo

Copy link
Copy Markdown

@modal-pr-review-automation modal-pr-review-automation bot left a comment

Choose a reason for hiding this comment

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

Approved 👍. @shababo will follow-up review this.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

# With all this in place, we are ready to define our high-performance, low-latency
# LFM 2 inference server.

app = modal.App("examples-lfm-snapshot")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 App name uses examples- prefix instead of example-

The app is named "examples-lfm-snapshot" at lfm_snapshot.py:268, but every other app in the llm-serving/ directory uses the "example-" prefix (singular): "example-vllm-inference", "example-vllm-low-latency", "example-sglang-snapshot", etc. The CLAUDE.md guidelines also specify example- prefix with kebab-case. The __main__ block at lfm_snapshot.py:507 correctly references the same "examples-lfm-snapshot" string, so this won't cause a runtime mismatch, but it breaks the naming convention.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


MINUTES = 60

MODEL_NAME = os.environ.get("MODEL_NAME", "LiquidAI/LFM2-8B-A1B")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 Model revision is not pinned, unlike other examples

The internal CLAUDE.md guidelines explicitly state: "Always pin model revisions to avoid surprises when upstream repos update". The vllm_low_latency.py example pins MODEL_REVISION and passes --revision to the vLLM CLI (vllm_low_latency.py:69-71, vllm_low_latency.py:276-277). This example does not pin a revision and does not pass --revision in the vLLM command at lines 296-317. If LiquidAI pushes a breaking update to their HuggingFace repo, this example could break silently.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +314 to +315
"--max-cudagraph-capture-size",
f"{MAX_INPUTS}",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 --max-cudagraph-capture-size CLI flag may not exist in vLLM v0.15.1

The vLLM serve command at lines 314-315 uses --max-cudagraph-capture-size, but this flag name doesn't appear in any other vLLM CLI invocation in the repo — only as a config dictionary key in gpt_oss_inference.py:136. Other vLLM examples pass cudagraph-related settings differently (e.g., through --compilation-config or not at all). Since the base image is vllm/vllm-openai:v0.15.1 (a future version I can't verify), I can't confirm whether this CLI flag is valid. If it isn't recognized, vLLM will fail to start.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@charlesfrye charlesfrye merged commit e3e8584 into main Feb 21, 2026
6 checks passed
@charlesfrye charlesfrye deleted the charlesfrye/liquidai-lfm branch February 21, 2026 02:41
@shababo
Copy link
Copy Markdown
Collaborator

shababo commented Feb 25, 2026

lgtm

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.

2 participants