Skip to content

Add specs/model_spec.md and enrich vgg11.py as agent reference material#9

Open
drewoldag wants to merge 2 commits intomainfrom
claude/create-model-spec-guide-U3pym
Open

Add specs/model_spec.md and enrich vgg11.py as agent reference material#9
drewoldag wants to merge 2 commits intomainfrom
claude/create-model-spec-guide-U3pym

Conversation

@drewoldag
Copy link
Copy Markdown
Collaborator

  • Create specs/ directory with model_spec.md: a dense, agent-targeted
    reference covering the @hyrax_model decorator, constructor contract,
    all five batch methods, prepare_inputs, TOML config, Hyrax wiring,
    and common pitfalls
  • Expand vgg11.py class/method docstrings so the file serves as the
    canonical annotated example agents are directed to read

https://claude.ai/code/session_01535J3bEqioGhVTVR7sdEN6

claude added 2 commits April 12, 2026 22:49
- Create specs/ directory with model_spec.md: a dense, agent-targeted
  reference covering the @hyrax_model decorator, constructor contract,
  all five batch methods, prepare_inputs, TOML config, Hyrax wiring,
  and common pitfalls
- Expand vgg11.py class/method docstrings so the file serves as the
  canonical annotated example agents are directed to read

https://claude.ai/code/session_01535J3bEqioGhVTVR7sdEN6
Copilot AI review requested due to automatic review settings April 13, 2026 00:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds agent-facing reference documentation for building Hyrax-compatible external PyTorch models, and upgrades the VGG11 example’s docstrings to serve as the annotated “canonical example” referenced by that spec.

Changes:

  • Added specs/model_spec.md describing the @hyrax_model contract, required batch methods, prepare_inputs, config conventions, and common pitfalls.
  • Expanded docstrings in src/external_hyrax_example/models/vgg11.py to document Hyrax integration requirements and per-batch method responsibilities.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/external_hyrax_example/models/vgg11.py Enriched docstrings/comments to explain Hyrax model integration and batch-method expectations.
specs/model_spec.md New consolidated spec for agents implementing Hyrax-compatible models.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +40 to +42
data_sample: A single batch tuple ``(images, labels)`` produced by
the dataset's ``prepare_inputs`` method. The first element must
be a float tensor of shape ``(N, C, H, W)``. The channel count
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says data_sample is produced by the dataset's prepare_inputs method, but none of the datasets in this package define prepare_inputs (and this model defines VGG11.prepare_inputs). Consider rewording to reflect that Hyrax/model prepare_inputs produces the batch tuple passed as data_sample.

Suggested change
data_sample: A single batch tuple ``(images, labels)`` produced by
the dataset's ``prepare_inputs`` method. The first element must
be a float tensor of shape ``(N, C, H, W)``. The channel count
data_sample: A single batch tuple ``(images, labels)`` prepared for
the model by Hyrax via ``prepare_inputs``. The first element must
be a float tensor of shape ``(N, C, H, W)``. The channel count

Copilot uses AI. Check for mistakes.
# Infer input channels from the sample rather than hard-coding them.
# data_sample[0] is a batch of images with shape (N, C, H, W).
image_sample = data_sample[0]
batch_size, self.in_channels, width, height = image_sample.shape
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image_sample.shape is documented as (N, C, H, W), but the unpacking uses width, height for the last two dims (and neither variable is used). Renaming these to height, width or using _ placeholders would avoid confusion about which dimension is which.

Suggested change
batch_size, self.in_channels, width, height = image_sample.shape
_, self.in_channels, _, _ = image_sample.shape

Copilot uses AI. Check for mistakes.

def __init__(self, config, data_sample=None):
"""Basic initialization with architecture definition"""
"""Initialise the VGG11 architecture.
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling inconsistency: this docstring uses "Initialise" while other docstrings in the package use "Initialize" (e.g. Galaxy10Dataset). Consider standardizing to "Initialize" for consistency.

Suggested change
"""Initialise the VGG11 architecture.
"""Initialize the VGG11 architecture.

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.00%. Comparing base (9ed73b5) to head (93a9a1e).

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #9   +/-   ##
=======================================
  Coverage   25.00%   25.00%           
=======================================
  Files           2        2           
  Lines          72       72           
=======================================
  Hits           18       18           
  Misses         54       54           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants