Serving, inference, v1 web-app with minimal front-end - #3
Conversation
…t vLLM doesn't introduce conflicts to deps
…toModel in config, separating AutoModel (no head) with AutoModelForCausalLM (head), more verbose params in MyConfig and config.json, add compatible hyperparam names, add input_embeds compatibility support
…ake it compatible to newer transformer versions
…_special_tokens format
…zer from HF repo instead of building it manually, remove debugging logic from chat func, more minor refactoring
… top_p_sampling, input preprocessing, response generation
… port/ip args in uvicorn run as it's overridden by fastapi launch script
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4c22c0f9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [tool.setuptools.packages.find] | ||
| include = ["post_train*", "pre_train*", "tokenizer*"] | ||
| exclude = ["checkpoints*", "data*", "dist*", "tests*", "trainer_output*", "wandb*"] | ||
| include = ["training*"] |
There was a problem hiding this comment.
The new setuptools discovery pattern only includes training*, but this repo’s importable modules are pre_train, post_train, tokenizer, and my_gpt_model at the project root. Building/installing this package will therefore omit the actual runtime modules, causing ModuleNotFoundError outside a source checkout (CI currently hides this via sys.path tweaks in tests). Please include the real package prefixes here, as in the previous root pyproject.toml.
Useful? React with 👍 / 👎.
| if resources is None: | ||
| raise HTTPException(status_code=503, detail="Model is not loaded.") | ||
|
|
||
| response = generate_response( |
There was a problem hiding this comment.
generate_response(...) is a long-running synchronous inference call, but it is executed directly inside an async FastAPI handler. Under concurrent traffic, this blocks the event loop for the duration of generation, delaying all other requests served by that worker. Run inference in a threadpool (or make the route sync) so long generations do not stall unrelated requests.
Useful? React with 👍 / 👎.
…to rely on normal package imports only
… inference optimizations are in place vs after
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51f5752ec7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| position_ids=position_ids, | ||
| **kwargs, | ||
| ) | ||
| logits = self.linear(hidden_states) |
There was a problem hiding this comment.
TransformerLM.forward_hidden_states() now returns a tuple (hidden_states, past_key_values), but forward() passes that tuple directly into self.linear, which expects a tensor. Any direct call to TransformerLM.forward() will therefore fail at runtime with a type error instead of producing logits (this path is separate from the HF wrapper methods that call forward_hidden_states() explicitly).
Useful? React with 👍 / 👎.
| run: uv sync --locked | ||
|
|
||
| - name: Import app | ||
| run: uv run python -c "from main import app" |
There was a problem hiding this comment.
serving
In the serving job, this command runs under working-directory: serving and imports main, but serving/main.py uses absolute imports like from serving.inference import .... From that working directory, serving is not on sys.path, so this import check fails before tests execute unless extra path hacks are applied. This makes the CI gate brittle and can block all serving PRs.
Useful? React with 👍 / 👎.
1 participant
No description provided.