From da38195692525bd5240fe6e6e468194238993350 Mon Sep 17 00:00:00 2001 From: Hatem Helal Date: Tue, 19 Sep 2023 18:36:34 +0100 Subject: [PATCH] Adding contribution guide and enable copyright checks as a pre-commit hook (#91) * Adding contribution guide * adding pre-commit checks on copyright * and a badge of course * adding copyrights * updating text * Apply suggestions from code review Co-authored-by: Andrew Fitzgibbon * don't blame the os for the python version --------- Co-authored-by: Andrew Fitzgibbon --- .flake8 | 4 + .github/workflows/pre-commit.yaml | 14 ++++ .pre-commit-config.yaml | 7 ++ CONTRIBUTING.md | 91 ++++++++++++++++++++++ README.md | 1 + notebooks/plot_utils.py | 1 + pyscf_ipu/electron_repulsion/__init__.py | 1 + pyscf_ipu/exchange_correlation/__init__.py | 1 + pyscf_ipu/exchange_correlation/b88.py | 1 + pyscf_ipu/exchange_correlation/lyp.py | 1 + pyscf_ipu/exchange_correlation/vwn.py | 1 + pyscf_ipu/nanoDFT/__init__.py | 1 + pyscf_ipu/nanoDFT/sparse_ERI.py | 1 + pyscf_ipu/nanoDFT/symmetric_ERI.py | 1 + requirements_test.txt | 3 + 15 files changed, 129 insertions(+) create mode 100644 .flake8 create mode 100644 .github/workflows/pre-commit.yaml create mode 100644 .pre-commit-config.yaml create mode 100644 CONTRIBUTING.md diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000..8ffaa86 --- /dev/null +++ b/.flake8 @@ -0,0 +1,4 @@ +[flake8] + +copyright-check = True +copyright-author = Graphcore Ltd \ No newline at end of file diff --git a/.github/workflows/pre-commit.yaml b/.github/workflows/pre-commit.yaml new file mode 100644 index 0000000..bd27cfc --- /dev/null +++ b/.github/workflows/pre-commit.yaml @@ -0,0 +1,14 @@ +name: Pre-Commit Checks + +on: + pull_request: + push: + branches: [main] + +jobs: + pre-commit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v3 + - uses: pre-commit/action@v3.0.0 \ No newline at end of file diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..fcbad61 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,7 @@ +repos: + - repo: https://github.com/PyCQA/flake8 + rev: 6.0.0 + hooks: + - id: flake8 + args: [--select=C] + additional_dependencies: [flake8-copyright] \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..c96e85c --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,91 @@ +# Contributing to pyscf-ipu + +This project is still evolving but at the moment is focused around a high-performance and easily hackable implementation of Gaussian basis set DFT. +We hope this is useful for the generation of large-scale datasets needed +for training machine-learning models. We are interested in hearing any and +all feedback so feel free to raise any questions, bugs encountered, or enhancement requests as [Issues](https://github.com/graphcore-research/pyscf-ipu/issues). + +## Setting up a development environment +We recommend using the conda package manager as this can automatically enable +the Graphcore Poplar SDK. This is particularly useful in VS Code which can automatically +activate the conda environment in a variety of scenarios: +* visual debugging +* running quick experiments in an interactive Jupyter window +* using VS code for Jupyter notebook development. + +The following assumes that you have already set up an install of conda and that +the conda command is available on your system path. Refer to your preferred conda +installer: +* [miniforge installation](https://github.com/conda-forge/miniforge#install) +* [conda installation documentation](https://docs.conda.io/projects/conda/en/latest/user-guide/install/index.html). + +1. Create a new conda environment with the same python version as required by the Poplar SDK. + For example, on ubuntu 20 use `python=3.8.10` + ```bash + conda create -n pyscf-ipu python=3.8.10 + ``` + +2. Confirm that you have the Poplar SDK installed on your machine and store the location + in a temporary shell variable. The following will test that the SDK is found and + configured correctly: + ```bash + TMP_POPLAR_SDK=/path/to/sdk + source $TMP_POPLAR_SDK/enable + gc-monitor + +3. Activate the environment and make POPLAR_SDK a persistent environment variable. + ```bash + conda activate pyscf-ipu + conda env config vars set POPLAR_SDK=$TMP_POPLAR_SDK + ``` + +4. You have to reactivate the conda environment to use the `$POPLAR_SDK` + variable in the environment. + ```bash + conda deactivate + conda activate pyscf-ipu + ``` + +5. Setup the conda environment to automatically enable the Poplar SDK whenever + the environment is activated. + ```bash + mkdir -p $CONDA_PREFIX/etc/conda/activate.d + echo "source $POPLAR_SDK/enable" > $CONDA_PREFIX/etc/conda/activate.d/enable.sh + ``` + +6. Check that everything is working by once reactivating the pyscf-ipu + environment _in a new shell_ and calling `gc-monitor`: + ```bash + conda deactivate + conda activate pyscf-ipu + gc-monitor + +7. Install all required packages for developing JAX DFT: + ```bash + pip install -e ".[ipu,test]" + ``` + +8. Install the pre-commit hooks + ```bash + pre-commit install + ``` + +9. Create a feature branch, make changes, and when you commit them the + pre-commit hooks will run. + ```bash + git checkout -b feature + ... + git push --set-upstream origin feature + ``` + The last command will prints a link that you can follow to open a PR. + + +## Testing +Run all the tests using `pytest` +```bash +pytest +``` +We also use the nbmake package to check our notebooks work in the `IpuModel` environment. These checks can also be run on IPU hardware equiped machines e.g.: +```bash +pytest --nbmake --nbmake-timeout=3000 notebooks/nanoDFT-demo.ipynb +``` diff --git a/README.md b/README.md index 6455503..0de4b2c 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ [![notebook-tests](https://github.com/graphcore-research/pyscf-ipu/actions/workflows/notebooks.yaml/badge.svg)](https://github.com/graphcore-research/pyscf-ipu/actions/workflows/notebooks.yaml) [![nanoDFT CLI](https://github.com/graphcore-research/pyscf-ipu/actions/workflows/cli.yaml/badge.svg)](https://github.com/graphcore-research/pyscf-ipu/actions/workflows/cli.yaml) [![unit tests](https://github.com/graphcore-research/pyscf-ipu/actions/workflows/unittest.yaml/badge.svg)](https://github.com/graphcore-research/pyscf-ipu/actions/workflows/unittest.yaml) +[![pre-commit checks](https://github.com/graphcore-research/pyscf-ipu/actions/workflows/pre-commit.yaml/badge.svg)](https://github.com/graphcore-research/pyscf-ipu/actions/workflows/pre-commit.yaml) [**Installation guide**](#installation) | [**Example DFT Computations**](#example-dft-computations) diff --git a/notebooks/plot_utils.py b/notebooks/plot_utils.py index 1643e7d..cd7a817 100644 --- a/notebooks/plot_utils.py +++ b/notebooks/plot_utils.py @@ -1,3 +1,4 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. import numpy as np import matplotlib as mpl import matplotlib.pyplot as plt diff --git a/pyscf_ipu/electron_repulsion/__init__.py b/pyscf_ipu/electron_repulsion/__init__.py index e69de29..3fa68fc 100644 --- a/pyscf_ipu/electron_repulsion/__init__.py +++ b/pyscf_ipu/electron_repulsion/__init__.py @@ -0,0 +1 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. \ No newline at end of file diff --git a/pyscf_ipu/exchange_correlation/__init__.py b/pyscf_ipu/exchange_correlation/__init__.py index e69de29..3fa68fc 100644 --- a/pyscf_ipu/exchange_correlation/__init__.py +++ b/pyscf_ipu/exchange_correlation/__init__.py @@ -0,0 +1 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. \ No newline at end of file diff --git a/pyscf_ipu/exchange_correlation/b88.py b/pyscf_ipu/exchange_correlation/b88.py index b37e69c..8775adf 100644 --- a/pyscf_ipu/exchange_correlation/b88.py +++ b/pyscf_ipu/exchange_correlation/b88.py @@ -1,3 +1,4 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. # The functional definition in this file was ported to Python # from XCFun, which is Copyright Ulf Ekström and contributors 2009-2020 # and provided under the Mozilla Public License (v2.0) diff --git a/pyscf_ipu/exchange_correlation/lyp.py b/pyscf_ipu/exchange_correlation/lyp.py index fec0063..464dfd4 100644 --- a/pyscf_ipu/exchange_correlation/lyp.py +++ b/pyscf_ipu/exchange_correlation/lyp.py @@ -1,3 +1,4 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. # The functional definition in this file was ported to Python # from XCFun, which is Copyright Ulf Ekström and contributors 2009-2020 # and provided under the Mozilla Public License (v2.0) diff --git a/pyscf_ipu/exchange_correlation/vwn.py b/pyscf_ipu/exchange_correlation/vwn.py index 308b9ac..5c96e79 100644 --- a/pyscf_ipu/exchange_correlation/vwn.py +++ b/pyscf_ipu/exchange_correlation/vwn.py @@ -1,3 +1,4 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. # The functional definition in this file was ported to Python # from XCFun, which is Copyright Ulf Ekström and contributors 2009-2020 # and provided under the Mozilla Public License (v2.0) diff --git a/pyscf_ipu/nanoDFT/__init__.py b/pyscf_ipu/nanoDFT/__init__.py index 8e91d5e..13a97d5 100644 --- a/pyscf_ipu/nanoDFT/__init__.py +++ b/pyscf_ipu/nanoDFT/__init__.py @@ -1 +1,2 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. from .nanoDFT import * diff --git a/pyscf_ipu/nanoDFT/sparse_ERI.py b/pyscf_ipu/nanoDFT/sparse_ERI.py index d5d3cce..7c2d3ae 100644 --- a/pyscf_ipu/nanoDFT/sparse_ERI.py +++ b/pyscf_ipu/nanoDFT/sparse_ERI.py @@ -1,3 +1,4 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. import numpy as np import jax.numpy as jnp import os diff --git a/pyscf_ipu/nanoDFT/symmetric_ERI.py b/pyscf_ipu/nanoDFT/symmetric_ERI.py index 7abcb03..abd9df0 100644 --- a/pyscf_ipu/nanoDFT/symmetric_ERI.py +++ b/pyscf_ipu/nanoDFT/symmetric_ERI.py @@ -1,3 +1,4 @@ +# Copyright (c) 2023 Graphcore Ltd. All rights reserved. import numpy as np import jax.numpy as jnp import os diff --git a/requirements_test.txt b/requirements_test.txt index 9d07eb3..f5e676f 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -6,3 +6,6 @@ # requirements_ipu.txt for ipu backend configuration pytest nbmake +pre-commit +flake8 +flake8-copyright