Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
130 lines (100 loc) · 5.9 KB

CONTRIBUTING.md

File metadata and controls

130 lines (100 loc) · 5.9 KB

Contributing

Thank you for your interest in contributing to this project! This is a research codebase for the Ai2 Climate Modeling group. While we welcome external users, please note that we do not guarantee backwards compatibility or supporting features that don't align with our research priorities.

Reporting Issues

We welcome bug reports and feature requests. Please open an issue to:

  • Report bugs you encounter
  • Suggest new features or improvements

Submitting Changes

Bug Fixes

We welcome pull requests that fix bugs. Please include a clear description of the issue and how your fix addresses it. Ideally, your pull request should include a unit test that would fail without your bug fix.

New Features

If you're an external user and want to implement a new feature, please open an issue first to discuss it with us. This helps ensure the feature aligns with the project's direction and prevents wasted effort.

Code Review

Automated CI tests must pass before your pull request will be reviewed. All pull requests will be reviewed for:

  • Correctness and functionality
  • Code style, type-hinting and consistency
  • Alignment with project goals
  • Responsibility isolation and abstraction level consistency
  • Appropriate test coverage

Please be responsive to feedback during the review process.

Review comment conventions

One guide our team finds helpful is Conventional Comments. Reviewers label comments to indicate priority:

  • Issue: Must be addressed before merge.
  • Suggestion (optional): Worth considering but non-blocking.
  • Question: Seeking clarification; may or may not require changes.
  • nit: Minor style preference; does not require re-review.

PR scope

  • Keep PRs focused. Split cleanly separable changes into separate PRs.
  • Non-blocking suggestions are often deferred to follow-on PRs.
  • Config changes typically need multiple rounds of review.

Developing

You can install a developer environment using the conda package manager with make create_environment.

Install pre-commit hooks to ensure linting and type-hinting requirements are met before committing code: pre-commit install.

You can run local tests with pytest. As a shortcut for running only faster unit tests, use make test_very_fast from the root of the repository.

Internal Development

If you are making changes directly to ai2cm/ace, please follow these internal development guidelines.

When making new branches, use the naming convention: <type>/<short-description>, where <type> is one of:

  • feature: Any new functionality in the repository including workflows and scripts but not including configurations. Should be the "default" type if it's unclear which to use.
  • refactor: No changes to features, just code restructuring or simplification.
  • fix: Bug fixes.
  • exp: Branch used for running experiments that is likely not intended to merge with mainline code. Functionality changes in these branches should first be PR'd using a feature branch.
  • config: Changes to baseline and experimental configurations under config/.
  • scripts: Changes isolated to a single script under scripts/ and subject to less rigorous review than changes to core code.
  • docs: Documentation changes only.

Code Guidelines

For general Python style we follow the Google Python Style Guide. The sections below cover what matters most in this project. See AGENTS.md for more detailed guidelines on naming, configuration, code organization, and other topics.

Design: isolate responsibilities

When designing a change, think first about what absolutely must change, then about what level of abstraction that change could be handled in. Choose the option that splits the concern across the fewest levels of abstraction. When a decision changes in the future, as little code as possible should need to be touched. This does not mean minimizing the number of lines changed — you may need to modify APIs at several levels to properly isolate a feature into one level of abstraction.

  • Prefer polymorphism over type-checking. If you see if isinstance(x, A) ... elif isinstance(x, B) ... chains, the behavior should be a method on the types being checked.
  • Keep functions at one level of abstraction. Don't mix high-level orchestration with low-level details. Extract helpers when needed.
  • Centralize cross-cutting concerns. Only Distributed should access torch.distributed; other modules call dist.method(). Training concerns (DDP, weight freezing, loss config) stay in training code.
  • Facade-first refactors. When refactoring across multiple PRs, implement the new class internally, keep the old class as a translation layer, then swap in a final PR.

Testing

  • Test helpers over copy-paste. Create helper functions to build common test fixtures. If the same setup appears 3+ times, extract a helper. Prefer explicit helper functions over pytest fixtures, which can become unwieldy; use fixtures only when sharing scope across tests is valuable.
  • Demonstrate bugs with failing tests. When fixing a bug, add a test that fails without the fix, then fix it.
  • Test behavior, not implementation. If a test re-implements the logic it's testing, it isn't actually verifying anything. Prefer tests that cover important user-story-level behavior over tests that lock down subjective API details, since the latter make it harder to evolve interfaces. Both have a place, but use API-level tests in moderation.
  • Use xfail for known bugs. Mark known issues with pytest.mark.xfail rather than skipping them silently.
  • Exercise meaningful values. Don't use all-ones for area weights or trivial shapes that hide real bugs.
  • Regression tests for checkpoints. Maintain regression checkpoints from specific model releases. Use strict=True for state dict loading.