Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new inference model infrastructure for AMS, implementing BaseModel and InferenceModel classes that provide a clean, move-only API for loading and executing TorchScript models. The changes include comprehensive error handling via a new AMSError system, extensive test coverage with generated test models, and refactored Python model generation scripts to support both legacy and new model formats.
Key changes:
- New
BaseModelandInferenceModelclasses with move-only semantics for safer model lifecycle management - Error handling infrastructure with
AMSErrorandAMSExpected<T>types - Comprehensive test suite covering model loading, type conversion, inference, and error scenarios
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/AMSlib/ml/Model.hpp | Declares BaseModel and InferenceModel classes with device/dtype management and inference APIs |
| src/AMSlib/ml/Model.cpp | Implements model loading from TorchScript files with AMS metadata extraction |
| src/AMSlib/ml/AbstractModel.hpp | Defines AbstractModel descriptor class for model metadata |
| src/AMSlib/ml/AbstractModel.cpp | Implements AbstractModel with JSON and path-based construction |
| src/AMSlib/include/AMSError.hpp | Introduces AMSError and AMSExpected for typed error handling |
| src/AMSlib/wf/fmt_helpers.hpp | Adds fmt formatter for std::filesystem::path |
| src/AMSlib/wf/debug.h | Updates header guard to pragma once and adds amsDebug() helper |
| tests/AMSlib/torch/test_model.cpp | Comprehensive tests for BaseModel loading, type checking, and conversion |
| tests/AMSlib/torch/test_inference_model.cpp | Tests for InferenceModel inference operations and error propagation |
| tests/AMSlib/torch/CMakeLists.txt | Registers new test executables for model tests |
| tests/AMSlib/models/generate_base_models.py | Python script to generate test models including failing model cases |
| tests/AMSlib/models/ams_model.py | Shared utilities for creating AMS-compatible TorchScript models |
| tests/AMSlib/models/generate.py | Updated to use shared ams_model utilities |
| tests/AMSlib/models/generate_linear_model.py | Updated to use shared ams_model utilities |
| tests/AMSlib/models/generate.sh | Invokes new generate_base_models.py script |
| tests/AMSlib/models/CMakeLists.txt | Adds generate_base_models.py to build dependencies |
| tests/AMSlib/CMakeLists.txt | Updates Catch2 from v3.6.0 to v3.11.0 |
| src/AMSlib/CMakeLists.txt | Adds Model.cpp and AbstractModel.cpp sources, links fmt and tl::expected |
| CMakeLists.txt | Fetches fmt and tl-expected dependencies, sets CMP0135 policy |
| cmake/FetchAndAddFmt.cmake | Adds fmt library fetching/discovery logic |
| cmake/FetchAndAddTlExpected.cmake | Adds tl-expected library fetching/discovery logic |
| .github/workflows/ci.yml | Adds git safe.directory configuration and formatting cleanup |
| .github/workflows/cpp-linter.yml | Adds git safe.directory configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e7d87f0 to
316a8bf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Depends on #159