Conversation
Reorganize the README and getting-started guide around quick native and Dev Container workflows. Document setup, branch selection, dependency overrides, container rebuilds, targeted tests, published images, and NERSC usage. Give Dev Container Compose projects per-user names to prevent shared Docker daemon collisions, and refresh the published image matrix and examples.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77c2e329a7
ℹ️ 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".
| devcontainer up | ||
| devcontainer exec bash |
There was a problem hiding this comment.
Pass the workspace folder to Dev Container commands
Anyone copying this quick start will invoke the CLI without its workspace selector, so devcontainer up/exec cannot identify the checkout and exit instead of starting or entering the container. The CLI's official examples use devcontainer up --workspace-folder <path> and devcontainer exec --workspace-folder <path> ... (Dev Container CLI “Try it”); add --workspace-folder . here and to the repeated commands in docs/getting-started.md.
Useful? React with 👍 / 👎.
| If you plan to run GPU-backed code, install the [NVIDIA Container | ||
| Toolkit](https://docs.nvidia.com/datacenter/cloud-native/container-toolkit/latest/install-guide.html) | ||
| on the host. |
There was a problem hiding this comment.
Grant GPU access to the Dev Container service
For developers following this section to run the documented GPU-backed tests, installing the NVIDIA Container Toolkit is not sufficient to attach a GPU to a Compose service. The inspected .devcontainer/compose.devcontainer.yaml has neither gpus nor a device reservation, so the resulting container has no GPU access and the test workflow fails; Docker's Compose documentation requires an explicit GPU reservation. Add GPU allocation to the service or document a working override.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR reorganizes Simphony’s top-level and getting-started documentation around two primary developer workflows (native build vs Dev Container) and refreshes the guidance for published container images and targeted testing. It also tweaks the Dev Container Compose configuration to reduce Docker daemon collisions across users.
Changes:
- Restructures
README.mdquick start into source build, Dev Container, published image, and Spack install flows, and updates the published image matrix. - Rewrites
docs/getting-started.mdto emphasize Dev Container development/testing, dependency version selection, container lifecycle steps, and NERSC usage. - Sets a per-user Docker Compose project name in
.devcontainer/compose.devcontainer.yamlto avoid shared-daemon collisions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| README.md | Reorganized quick start sections, updated container commands, and refreshed published image matrix table. |
| docs/getting-started.md | Expanded Dev Container-first workflow documentation and added more explicit build/test and container lifecycle guidance. |
| .devcontainer/compose.devcontainer.yaml | Adds an explicit Compose project name intended to be per-user. |
Comments suppressed due to low confidence (2)
docs/getting-started.md:73
devcontainer up/devcontainer execare missing--workspace-folder .. Without it, the CLI may not locate the workspace (and therefore the correct container) depending on how the command is invoked.
devcontainer up
devcontainer exec bash
docs/getting-started.md:84
- This second container-recreate example also needs
--workspace-folder .to be consistent with the CLI requirements and to ensure it targets the current checkout's devcontainer.
devcontainer up --remove-existing-container
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| npm install -g @devcontainers/cli | ||
| git clone https://github.com/BNLNPPS/simphony.git | ||
| cd simphony | ||
| devcontainer up | ||
| devcontainer exec bash |
|
|
||
| ```shell | ||
| docker run --rm --gpus all ghcr.io/bnlnpps/simphony bash -lc 'simg4ox -g tests/geom/raindrop.gdml -m tests/run.mac' | ||
| devcontainer up --remove-existing-container |
| Skip this section if the default dependency versions meet your needs. The root | ||
| `.env` file selects the OS and toolchain used to build the Dev Container. | ||
| `.devcontainer/.env` links to it so Compose can discover the same settings. | ||
| Edit `.env` when you want to use another supported combination or experiment | ||
| with a new dependency version. |
Reorganize the README and getting-started guide around quick native and
Dev Container workflows. Document setup, branch selection, dependency
overrides, container rebuilds, targeted tests, published images, and
NERSC usage.
Give Dev Container Compose projects per-user names to prevent shared
Docker daemon collisions, and refresh the published image matrix and
examples.