Conversation
![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: c511c338ef
ℹ️ 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".
| auto results = handle->model->prog.eval(params); | ||
|
|
||
| // Process outputs | ||
| assert(outputs.size() == (size_t)batchSize); |
There was a problem hiding this comment.
In getOutput, the code calls prog.eval(params) and then immediately reads from policyResults, policyPassResults, valueResults, and scoreValueResults, but none of those buffers are ever filled from results. Since these vectors are initialized to zeros in createInputBuffers, every MIGraphX inference returns zero/default predictions regardless of the model, which breaks gameplay and analysis output for all runs using this backend.
Useful? React with 👍 / 👎.
| migraphx::shape input_shape( | ||
| handle->model->useFP16 ? migraphx::shape::half_type : migraphx::shape::float_type, | ||
| {(size_t)maxBatchSize, (size_t)numSpatialFeatures, (size_t)nnYLen, (size_t)nnXLen} | ||
| ); | ||
| params["input_spatial"] = migraphx::argument(input_shape, buffers->userInputBuffer.data()); |
There was a problem hiding this comment.
When FP16 is enabled, input_shape is declared as half_type but input_spatial is bound to float* memory (userInputBuffer.data()), so MIGraphX will interpret the buffer with the wrong element width/type. This produces corrupted inputs (or runtime failures) specifically when mgxUseFP16 is true; the backend should convert host inputs to migraphx::half before creating FP16 arguments, similar to the test helper paths.
Useful? React with 👍 / 👎.
| // For now, treat as ordinary residual block | ||
| // Full implementation would use gpoolConv, gpool pooling, and gpoolToBiasMul | ||
| (void)blockDesc; | ||
| return buildResidualBlock(builder, input, *(const ResidualBlockDesc*)&blockDesc); |
There was a problem hiding this comment.
buildGlobalPoolingResidualBlock reinterprets GlobalPoolingResidualBlockDesc as ResidualBlockDesc, but these structs are not layout-compatible (GlobalPoolingResidualBlockDesc has an extra modelVersion field before preBN). This misaligns all subsequent fields and feeds invalid layer descriptors into graph construction whenever a global-pooling block is encountered, leading to incorrect graphs or crashes.
Useful? React with 👍 / 👎.
1 participant
This pull request adds support for the MIGraphX backend, enabling KataGo to run neural network inference on AMD GPUs using the MIGraphX library. The changes update build scripts, documentation, and configuration files to integrate MIGraphX as a selectable backend alongside existing ones like CUDA, ROCm, and OpenCL. Users can now build and configure KataGo to use MIGraphX on compatible AMD hardware.
Build system and backend integration:
cpp/CMakeLists.txt, including logic to find and link MIGraphX libraries, set up include paths, and define theUSE_MIGRAPHX_BACKENDcompile flag. [1] [2] [3]cpp/program/setup.cppto recognize and initialize the MIGraphX backend. [1] [2]Documentation updates:
README.mdandCompiling.mdto document MIGraphX as a supported backend, describe its requirements, and explain its usage and performance characteristics. [1] [2] [3] [4] [5]Configuration and user guidance:
Runtime and program feedback:
cpp/main.cppto indicate when the MIGraphX backend is in use.These changes collectively make KataGo compatible with the MIGraphX backend, expanding support for AMD GPUs and providing users with more flexibility in hardware and backend selection.