Skip to content

Closing gaps with public release requirements - #134

Merged
andrewfayres merged 7 commits into
mainfrom
public-release-readiness
Oct 8, 2026
Merged

andrewfayres merged 7 commits into
mainfrom
public-release-readiness

Conversation

@andrewfayres

@andrewfayres andrewfayres commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Brings Apeiron in line with the BaseTemplate requirements for ModCon Base public repositories so we can release it publicly. The PR adds the missing community and governance files, migrates the project from Poetry to uv, and upgrades dependencies to clear the open Dependabot alerts.

Motivation & Context

  • Before this PR the repo had no CONTRIBUTING.md, CODE_OF_CONDUCT.md, SECURITY.md, CHANGELOG.md, issue templates, or AI-contribution policy, and no acknowledgment of DOE / Genesis Mission support.
  • It also covers @wildsm's compliance checklist and suggested cleanups from the earlier review.
  • @anagainaru confirmed that branch protection is enabled on main.
  • 122 Dependabot alerts were open (6 critical, 69 high).

Approach

The commits are best reviewed one at a time:

  1. Public-release files (73d3ebb, b7281b2)
    • Adds CONTRIBUTING.md (including BaseTemplate's AI/LLM-assisted contribution policy, verbatim), CODE_OF_CONDUCT.md, SECURITY.md (GitHub private vulnerability reporting), CHANGELOG.md, and issue templates.
    • Adds license, classifier, and URL metadata to pyproject.toml, and a Makefile.
    • Adds the DOE Office of Science / ASCR acknowledgment to the README and docs.
    • Also: PR-triggered CI, fixed README badges, the review's typo and stale-URL fixes, and a test file rename.
  2. get_optmizer → get_optimizer (d73501f)
    • Fixes the spelling without breaking existing harnesses. The old abstract method is overridden in user code, so a plain alias wouldn't work.
    • __init_subclass__ bridges subclasses that implement only the old name onto the new one, and warns when the class is defined.
  3. Dependency upgrade (ecd5ed1) — clears 121 of 122 Dependabot alerts.
  4. Poetry → uv (8f26ba1, 6c96c85)
    • The torch upgrade exposed a Poetry resolver quirk. torch 2.13 reaches nvidia-cublas through two paths with differently shaped markers, and Poetry locked two conflicting versions whose markers both apply on Linux. That made the lock uninstallable on Linux, which broke CI and poetry install for anyone working from source.
    • uv resolves a single version. It's also BaseTemplate's recommended tool.
  5. wandb fix (b2ad48b) — wandb 0.30 removed Run.get_url(), so WandBLogger.url raised AttributeError. It now uses Run.url, which exists across the whole supported range (wandb>=0.22). mypy caught this.

API / CLI Changes

  • BaseModelHarness.get_optimizer() — new; the correctly spelled abstract hook.
  • BaseModelHarness.get_optmizer() — deprecated; still works and emits a DeprecationWarning.
  • New make targets: install, lock, test, test-cov, lint, format, type-check, docs, clean.

Breaking Changes

No Python API breaks. Changes to the development workflow and environments:

  • Poetry → uv. Developers need uv; uv sync replaces poetry install and uv run replaces poetry run. poetry.lock is replaced by uv.lock.
  • torch >=2.13.0 and torchvision 0.28.x, up from 2.9 / 0.24.
    • torch 2.13's PyPI wheels target CUDA 13, which needs NVIDIA driver ≥580.
    • Its ROCm wheels need ROCm 7.1+, so the Frontier script now loads a ROCm 7.1 module instead of 6.4.

Security & Privacy

  • No secrets committed
  • Resolves 121 of 122 open Dependabot alerts. The one left, nltk GHSA-8mgp-746c-j5xp (high), has no published fix: 3.10.3 is the latest release and is still affected. nltk comes in transitively via evidently, and Apeiron never calls it directly. It should be dismissed as "no fix available" once this merges.
  • Adds SECURITY.md with a private reporting path.

Dependencies

  • Upgraded: torch 2.9.1 → 2.13.0, torchvision 0.24.1 → 0.28.0, transformers 5.8.1 → 5.17.0, mlflow 3.12.0 → 3.16.1, wandb 0.23.0 → 0.30.0, plus transitive upgrades (aiohttp, pillow, cryptography, gitpython, starlette, nltk, …).
  • Removed: black (unused; formatting is ruff format).
  • Build backend: poetry-core → uv_build. The built wheel ships the same files as before.

Testing Plan

  • Unit tests — the full suite passes in CI on Linux.
  • Docker image builds and its tests pass (Docker-test).
  • Locally: uv lock --check, ruff lint and format, mypy (0 issues), and make docs with warnings as errors all pass.
  • e2e: the examples job (MNIST and CIFAR training runs) only runs on manual dispatch and hasn't been run on this branch. It's the best end-to-end check of the torch 2.9 → 2.13 upgrade, so it's worth triggering before merge.
  • The HPC install scripts (Frontier, Perlmutter) haven't been run on the clusters. The Linux and ROCm 7.1 installs were dry-run resolved, but not executed.

Documentation

  • Docstrings updated
  • User docs / README updated
  • CHANGELOG entry

AI / LLM Assistance

  • AI/LLM tools were primarily used to generate code or artifacts in this PR (Claude Code)
  • I have reviewed all such output line-by-line and take full responsibility for its correctness, security, and licensing

Checklist

  • Code formatted (Ruff) → ruff format --check
  • Lint passes (Ruff) → ruff check .
  • Types pass (mypy) → mypy .
  • Tests pass (pytest)
  • Backward compatibility considered
  • Adequate comments for tricky parts
  • CI green

@wildsm wildsm left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR is not yet mark ready for review, but I wanted to capture some repo level checks to start. There are some suggested changes at the bottom. I will complete a full review once PR is marked ready

Compliance Checklist

This checklist is based on the BaseTemplate public-repository requirements and the current
repository checkout only. A checked box means the item is supported by evidence in the repo.
Unchecked boxes are items I could not confirm from the repo contents alone. Struck-through text
marks an item that does not apply to this repo's chosen tooling or structure.

Repository Basics

  • Public-facing README exists and is substantive. Evidence: README.md describes the project purpose, installation, running examples, configuration, docs, deployment, contributing, support, license, code of conduct, and security.
  • License file is present. Evidence: LICENSE exists, and the license is also declared in pyproject.toml and referenced in README.md.
  • Contributing guide is present. Evidence: CONTRIBUTING.md documents bug reports, feature requests, pull requests, style, docs, testing, and AI/LLM-assisted contributions.
  • Code of Conduct is present. Evidence: CODE_OF_CONDUCT.md is linked from README.md and CONTRIBUTING.md.
  • Security policy is present. Evidence: SECURITY.md provides a private vulnerability-reporting path and supported-version guidance.
  • Changelog is present. Evidence: CHANGELOG.md exists and is referenced from README.md and pyproject.toml.
  • Repository support / project acknowledgment is documented. Evidence: README.md includes the Genesis Mission acknowledgment.

Package Management

  • Dependency metadata is declared in a standard project file. Evidence: pyproject.toml contains project metadata, dependencies, dev dependencies, build-system configuration, and project URLs.
  • Dependencies are locked for reproducibility. Evidence: poetry.lock is committed and corresponds to the Poetry workflow documented in docs/installation.md.
  • Installation and development setup instructions are documented. Evidence: docs/installation.md and README.md describe poetry install, verification steps, and dependency usage.
  • Development commands are documented. Evidence: Makefile exposes install, test, lint, format, type-check, docs, and clean, and README.md summarizes them.
  • The package is structured as an installable Python project. Evidence: pyproject.toml declares src/apeiron as the package root, and the docs show the public import path apeiron.
  • uv.lock is present Not applicable: this repository standardizes on Poetry rather than uv, so poetry.lock is the relevant lockfile.

Documentation

Testing And CI

  • Test files are present. Evidence: tests/ contains unit and integration coverage for drift detection, trainers, logging, evaluation, config, and profiling.
  • CI workflow exists. Evidence: .github/workflows/build-test.yml runs lint, type checking, tests, and coverage upload.
  • A second workflow exists for image-based validation. Evidence: .github/workflows/build-image.yml builds the Docker image and runs pytest inside it.
  • Quality commands are documented and match CI intent. Evidence: Makefile and CONTRIBUTING.md both list lint, type-check, and test commands.
  • CI is configured to run on pull requests. The current workflow in .github/workflows/build-test.yml is triggered by push and workflow_dispatch, so I could not confirm PR-triggered checks from the repo.

Community Workflow

Notes

  • The package-management requirement is satisfied in principle through Poetry: the repo has a declared project file, a committed lockfile, and documented install instructions.
  • The main actionable compliance gap visible from the checkout is PR-triggered CI plus GitHub-side branch protection, because those are not verifiable or enforced from the repository files alone.

Action Items

  1. Add pull_request to .github/workflows/build-test.yml so the CI status is available on PRs.
  2. Enable branch protection on the default branch in GitHub and require the CI checks before merge.
  3. Confirm GitHub repository security features are enabled: Dependabot alerts, secret scanning, and code scanning.

Suggested Cleanup

Codex was used to facilitate this review, but I stand behind it.

Andrew Ayres and others added 5 commits September 29, 2026 09:10
- Run CI on pull_request so status checks report on PRs. The examples job
  stays gated to workflow_dispatch, so PRs only run the build job.
- Use the DOE Office of Science / ASCR acknowledgment wording in the README
  and docs landing page.
- Fix stale BaseSim_Framework clone URLs in the Frontier and Perlmutter
  deployment guides.
- Fix typos: "Froniter", "Requirested", "beecause".
- Rename tests/test_valiadation_tests.py to tests/test_validation_tests.py.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BaseModelHarness.get_optmizer() was a misspelled abstract method that users
override in their own harnesses, so it cannot be renamed with a simple alias:
the bridge has to work in both directions.

- get_optimizer() is now the abstract method the framework calls.
- get_optmizer() remains as a concrete deprecated shim that delegates to
  get_optimizer() and warns at call time, for external callers of the old name.
- __init_subclass__ detects a subclass that implements only the old spelling,
  aliases it onto get_optimizer() so it still satisfies the ABC, and warns at
  class-definition time.

A harness written against either spelling keeps working. Renames the in-repo
implementations (three example harnesses and the test harness) and the one
caller in continuous_trainer, and updates the docs, examples README, CLAUDE.md,
and the Claude/Codex skill files, two of which previously told authors to
preserve the misspelling.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Puts us in sync with BaseTemplate, and also resolves a dependency issue
caused by a quirk in how Poetry builds the dependency chain: with torch
2.13, Poetry locked two conflicting versions of nvidia-cublas and
nvidia-cuda-nvrtc under overlapping Linux markers, which broke installs
on Linux. uv resolves a single version of each.

Claude helped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@anagainaru

Copy link
Copy Markdown
Collaborator

I can confirm that Branch protection rules are enabled for the default branch.

@anagainaru anagainaru left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know this is still a draft, but I had some time to look over this. It's not great that we cannot use pip install apeiron. If we want to change the name this is the time, before we submit the SoftwareX paper. Or if we keep the name can we do something like pip install modcon-apeiron?

Comment on lines 42 to +51

def __init_subclass__(cls, **kwargs: Any) -> None:
"""
Bridge the deprecated ``get_optmizer`` spelling onto ``get_optimizer``.

A harness written against the old misspelled name keeps working: its
implementation is aliased onto the new name, so the framework only ever
has to call ``get_optimizer``.
"""
super().__init_subclass__(**kwargs)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure if we want to keep backwards compatibility.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think that we should for one release then drop it

Comment on lines +64 to +66


class TestGetOptimizerDeprecation:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is also not needed if we don't keep the deprecation.

Comment thread CHANGELOG.md
@@ -0,0 +1,90 @@
# Changelog

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this file really needed in root? Isn't this covered in the release notes?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Kinda. One benefit is it lives alongside the code so it's there for whoever pulls from source and not just releases. It also makes it easy to create the release notes (might be less meaningful in the world of AI).

@andrewfayres

Copy link
Copy Markdown
Collaborator Author

We can use genesis-apeiron, modcon-apeiron, or something like that on pypi

@andrewfayres
andrewfayres marked this pull request as ready for review October 6, 2026 14:22
@anagainaru

Copy link
Copy Markdown
Collaborator

Keeping the code is fine as long as we remember to remove it for the next release (not sure what the standard in python is to mark a code deprecated). This looks good to me, but Stefan or Nathan need to approve it.

@wildsm wildsm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for all the development, this was a great leap forward toward addressing https://github.com/AI-ModCon/BaseTemplate#required-elements-for-modcon-base-public-repositories

Some comments:

  • With the move from poetry to uv, I was reviewing lingering poetry occurrences. I think all is well, but note that there are poetry references in .codex/skills/install-apeiron/SKILL.md and .claude/skills/install-apeiron/SKILL.md. If desired, you could add a brief note toward the top each skill to clearly state that This skill can install Apeiron into projects managed by uv, Poetry, or pip. This repository itself uses uv.

  • PR-triggered CI: build-test.yml runs on push, pull_request, and workflow_dispatch, and includes lint, type checking, tests, and coverage. Remove the action item to add pull_request; it is already configured. Note that build-image.yml is push-only.

  • The repo tripped on the security policy since the tab mentioned in SECURITY.md was empty, but this PR should populate it (something to check after merge).

@wildsm

wildsm commented Oct 6, 2026

Copy link
Copy Markdown
Member

@anagainaru - An approving review from someone with write access is needed. Can you please formally review/complete review?

@wildsm

wildsm commented Oct 6, 2026

Copy link
Copy Markdown
Member

@andrewfayres - I think we are good to merge, congrats

@andrewfayres
andrewfayres merged commit ec43dca into main Oct 8, 2026
5 checks passed
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