Skip to content

Agents updates - #1711

Draft
tteggelit wants to merge 3 commits into
GoogleCloudPlatform:developfrom
tteggelit:agents_updates
Draft

Agents updates#1711
tteggelit wants to merge 3 commits into
GoogleCloudPlatform:developfrom
tteggelit:agents_updates

Conversation

@tteggelit

Copy link
Copy Markdown
Contributor

This update AGENTS.md to make it smaller (to reduce context consumption) and move aspects to relevant skills as opposed to being in the agent context. It updates the existing ramble-workspace-wizard to address some changes to workflow_manager (config: vs. variants:) and adds several new skills that can be used in more agentic workflows as needed.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces several new AI agent skill guides under .agents/skills/ and refactors AGENTS.md to reference these modular skills. Feedback on the changes highlights two main configuration corrections: first, updating the workflow manager declaration in the GCP Cluster Toolkit guide to use the variants: section instead of config:; second, adjusting the Spack integration guide to configure compilers via the explicit compiler: directive within software:packages: rather than inline with % in the spack_spec.

Comment on lines +105 to +107
ramble:
config:
workflow_manager: slurm

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.

medium

The workflow manager configuration should be declared under the variants: section instead of the config: section, to align with the updated schema where workflow_manager is a variant. Please also update the text on line 101 to refer to variants: workflow_manager: slurm.

Suggested change
ramble:
config:
workflow_manager: slurm
ramble:
variants:
workflow_manager: slurm

Comment on lines +63 to +66
openmpi4:
spack_spec: openmpi@4.1.5 %gcc12 +cuda
gromacs_pkg:
spack_spec: gromacs@2023.2 %gcc12 ^openmpi4

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.

medium

According to the compiler association guidelines, compilers should be configured strictly via the compiler: directive within the software:packages: entry, rather than specifying them inline with % in the spack_spec.

Suggested change
openmpi4:
spack_spec: openmpi@4.1.5 %gcc12 +cuda
gromacs_pkg:
spack_spec: gromacs@2023.2 %gcc12 ^openmpi4
openmpi4:
spack_spec: openmpi@4.1.5 +cuda
compiler: gcc12
gromacs_pkg:
spack_spec: gromacs@2023.2 ^openmpi4
compiler: gcc12

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.

1 participant