Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add contribution guidelines #71

Merged
merged 22 commits into from
May 12, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ jobs:
matrix:
julia-version: ['1']
julia-arch: [x64, x86]
os: [ubuntu-latest, macOS-latest]
exclude:
- os: macOS-latest
julia-arch: x86
os: [ubuntu-latest]
fail-fast: false
steps:
- uses: actions/checkout@v2
Expand Down
41 changes: 41 additions & 0 deletions .github/workflows/format_check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
name: format-check

on:
push:
branches:
- 'main'
tags: '*'
pull_request:

jobs:
build:
runs-on: ${{ matrix.os }}
strategy:
matrix:
julia-version: [1.6.0]
julia-arch: [x86]
os: [ubuntu-latest]
steps:
- uses: julia-actions/setup-julia@latest
with:
version: ${{ matrix.julia-version }}

- uses: actions/checkout@v2
- name: Install JuliaFormatter and format
# This will use the latest version by default but you can set the version like so:
#
# julia -e 'using Pkg; Pkg.add(PackageSpec(name="JuliaFormatter", version="0.13.0"))'
run: |
julia -e 'using Pkg; Pkg.add(PackageSpec(name="JuliaFormatter"))'
julia -e 'using JuliaFormatter; format(".", verbose=true)'
- name: Format check
run: |
julia -e '
out = Cmd(`git diff --name-only`) |> read |> String
if out == ""
exit(0)
else
@error "Some files have not been formatted !!!"
write(stdout, out)
exit(1)
end'
43 changes: 43 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Contribution Guide

We very happily accept contributions from the community to make our packages better! For the smoothest experience, please read this document and follow the guidelines and we can hopefully get your PR merged in a jiffy! We've tried to keep the guidelines lightweight, reasonable, and not too onerous. :)

(Don't know what a PR is? Know how to write Julia code but never contributed to a package before? Refer to the [Getting Started](#getting-started) section further on down the page.)

Thanks to the [OpenMM contribution guide](https://github.com/openmm/openmm/blob/master/CONTRIBUTING.md) and the [SciML ColPrac document](http://colprac.sciml.ai), which were the main inspirations/starting points for the suggestions contained herein.

## Guidelines

* Commit frequently and make the commit messages detailed! Ideally specifying directory/file as well as nature of changes. A sample commit message format could be:
```
directory of file affected: changes introduced

...commit message explicitly stating the changes made. this should be concise, and crisp enough, that the maintainers must be able to understand the changes this commit introduces without having to go through the diff-logs...

Signed-off/Co-authored with/Suggested-by messages for credit where it's due
```
* In general, unless a change is very minor (e.g. fixing a typo), open an issue before opening a pull request that fixes that issue. This allows open discussion, collaboration, and prioritization of changes to the code. Please also label the issue appropriately. We use a set of labels that is slightly expanded from the [GitHub standard set](https://docs.github.com/en/github/managing-your-work-on-github/managing-labels#about-default-labels):

| Label | Description |
| ------------- | ------------- |
| `breaking` | Indicates a pull request that introduces breaking changes |
| `bug` | Indicates an unexpected problem or unintended behavior |
| `documentation` | Indicates a need for improvements or additions to documentation |
| `duplicate` | Indicates similar issues or pull requests |
| `enhancement` | Indicates new feature requests |
| `good first issue` | Indicates a good issue for first-time contributors |
| `help wanted` | Indicates that a maintainer wants help on an issue or pull request |
| `invalid` | Indicates that an issue or pull request is no longer relevant |
| `longterm` | Indicates a feature that we intend to implement, but is not high-priority right now (generally only to be used by maintainers) |
| `performance` | Indicates an issue/PR to improve code performance. |
| `priority` | Indicates an issue that is high-priority (generally only to be used by maintainers) |
| `question` | Indicates that an issue or pull request needs more information |
| `wontfix` | Indicates that work won't continue on an issue or pull request |

* If you are adding/changing features, make sure to add/update tests (DO NOT comment out tests!) and documentation accordingly! Ideally, if relevant, include example usage.
* Keep things modular! If you are fixing/adding multiple things, do so via separate issues/PR's to streamline review and merging.
* It is recommended that you set up [JuliaFormatter](https://domluna.github.io/JuliaFormatter.jl/dev/) (also see [VSCode extension](https://marketplace.visualstudio.com/items?itemName=singularitti.vscode-julia-formatter)). A GitHub action will check that code adheres to the style guide.

## Getting Started
Copy link
Member

Choose a reason for hiding this comment

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

How necessary is this in CONTRIBUTING.md? I'm assuming that stuff like running git clone ..., activating a Julia environment, etc would be mentioned here.
But wouldn't the people who would be looking at this file already have the essential know-how of doing all that?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the answer to this depends upon how many people you think are out there who can write decent Julia code, might be interested in contributing some of their code to a package, but haven't actually developed packages before. Given that I was such a person relatively recently (and was one in Python a couple years before that), I tend to think there are a fair number, but I may be biased due to personal experience... ;) At any rate, I don't think it hurts to have it there...

Copy link
Member Author

Choose a reason for hiding this comment

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

Rather than actually writing a new guide, I decided to just compile some other nice resources.


Coming...
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ Documentation is in progress [over here](https://aced-differentiate.github.io/At

## Getting Started

1. Clone this package to wherever you want to play.
1. To install the latest tagged version, in your Julia REPL, do `]install AtomicGraphNets`. However, you can also play with the latest version on the `main` branch by skipping to step 2 and then doing `]install /path/to/repo` where you replace the dummy path with the location of your clone.

rkurchin marked this conversation as resolved.
Show resolved Hide resolved
2. Go and try out the example in examples/example1/ – it has its own README file with detailed instructions.
2. Clone this package to wherever you want to play.

* more network architectures (see issues for some ideas)
3. Go and try out the example in examples/example1/ – it has its own README file with detailed instructions.

Copy link
Member

Choose a reason for hiding this comment

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

I think it might help to put a quick note on how to actually use the package here (like ] add AtomicGraphNets, and variants).

## Contact
Please feel free to fork and play, and reach out here on GitHub or to rkurchin [at] cmu [dot] edu with suggestions, etc.!
## Contributing
We welcome community contributions! Please refer to [contribution guide](CONTRIBUTING.md) for suggestions on how to go about things.

## Acknowledgements
Many thanks to [Dhairya Gandhi](https://github.com/DhairyaLGandhi) for helping out with some adjoints to actually make these layers trainable! :D
51 changes: 25 additions & 26 deletions docs/make.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,31 @@ if haskey(ENV, "DOCSARGS")
end

makedocs(
sitename = "AtomicGraphNets.jl",
modules = [AtomicGraphNets],
pages = Any[
"Home" => "index.md",
"Basic Graph Theory" => "graph_theory.md",
"GCNNs" => "gcnns.md",
"Comparison with cgcnn.py" => "comparison.md",
"Examples" => Any[
"Example 1" => "examples/example_1.md",
"Example 2" => "examples/example_2.md"],
"Functions" => Any[
"Layers" => "functions/layers.md",
"Models" => "functions/models.md"],
"Changelog" => "changelog.md"
],
format = Documenter.HTML(
# Use clean URLs, unless built as a "local" build
prettyurls = !("local" in ARGS),
canonical = "https://aced-differentiate.github.io/AtomicGraphNets.jl/stable/",
),
linkcheck = "linkcheck" in ARGS,
sitename = "AtomicGraphNets.jl",
modules = [AtomicGraphNets],
pages = Any[
"Home"=>"index.md",
"Basic Graph Theory"=>"graph_theory.md",
"GCNNs"=>"gcnns.md",
"Comparison with cgcnn.py"=>"comparison.md",
"Examples"=>Any[
"Example 1"=>"examples/example_1.md",
"Example 2"=>"examples/example_2.md",
],
"Functions"=>Any["Layers"=>"functions/layers.md", "Models"=>"functions/models.md"],
"Changelog"=>"changelog.md",
],
format = Documenter.HTML(
# Use clean URLs, unless built as a "local" build
prettyurls = !("local" in ARGS),
canonical = "https://aced-differentiate.github.io/AtomicGraphNets.jl/stable/",
),
linkcheck = "linkcheck" in ARGS,
)
deploydocs(
repo = "github.com/aced-differentiate/AtomicGraphNets.jl.git",
target = "build",
branch = "gh-pages",
devbranch = "main",
push_preview = true
repo = "github.com/aced-differentiate/AtomicGraphNets.jl.git",
target = "build",
branch = "gh-pages",
devbranch = "main",
push_preview = true,
)
27 changes: 20 additions & 7 deletions examples/1_formation_energy/formation_energy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ num_bins = [18, 9, 4, 16, 10, 10]
num_features = sum(num_bins) # we'll use this later
logspaced = [false, false, false, true, true, false]
# returns actual vectors (in a dict with keys of elements) plus Vector of AtomFeat objects describing featurization metadata
atom_feature_vecs, featurization = make_feature_vectors(features, nbins=num_bins, logspaced=logspaced)
atom_feature_vecs, featurization =
make_feature_vectors(features, nbins = num_bins, logspaced = logspaced)

# model hyperparameters – keeping it pretty simple for now
num_conv = 3 # how many convolutional layers?
Expand All @@ -39,12 +40,12 @@ num_hidden_layers = 1 # how many fully-connected layers after convolution and po
opt = ADAM(0.001) # optimizer

# dataset...first, read in outputs
info = CSV.read(string(datadir,prop,".csv"), DataFrame)
info = CSV.read(string(datadir, prop, ".csv"), DataFrame)
y = Array(Float32.(info[!, Symbol(prop)]))

# shuffle data and pick out subset
indices = shuffle(1:size(info,1))[1:num_pts]
info = info[indices,:]
indices = shuffle(1:size(info, 1))[1:num_pts]
info = info[indices, :]
output = y[indices]

# next, make graphs and build input features (matrices of dimension (# features, # nodes))
Expand All @@ -70,13 +71,25 @@ train_data = zip(train_input, train_output)

# build the model
println("Building the network...")
model = build_CGCNN(num_features, num_conv=num_conv, atom_conv_feature_length=crys_fea_len, pooled_feature_length=(Int(crys_fea_len/2)), num_hidden_layers=1)
model = build_CGCNN(
num_features,
num_conv = num_conv,
atom_conv_feature_length = crys_fea_len,
pooled_feature_length = (Int(crys_fea_len / 2)),
num_hidden_layers = 1,
)

# define loss function and a callback to monitor progress
loss(x,y) = Flux.Losses.mse(model(x), y)
loss(x, y) = Flux.Losses.mse(model(x), y)
evalcb() = @show(mean(loss.(test_input, test_output)))
evalcb()

# train
println("Training!")
@epochs num_epochs Flux.train!(loss, params(model), train_data, opt, cb = Flux.throttle(evalcb, 5))
@epochs num_epochs Flux.train!(
loss,
params(model),
train_data,
opt,
cb = Flux.throttle(evalcb, 5),
)
26 changes: 20 additions & 6 deletions examples/2_qm9/qm9.jl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ num_bins = [18, 9, 4, 16, 10, 10]
num_features = sum(num_bins) # we'll use this later
logspaced = [false, false, false, true, true, false]
# returns actual vectors (in a dict with keys of elements) plus Vector of AtomFeat objects describing featurization metadata
atom_feature_vecs, featurization = make_feature_vectors(features, nbins=num_bins, logspaced=logspaced)
atom_feature_vecs, featurization =
make_feature_vectors(features, nbins = num_bins, logspaced = logspaced)

# model hyperparameters – keeping it pretty simple for now
num_conv = 5 # how many convolutional layers?
Expand All @@ -45,8 +46,8 @@ num_hidden_layers = 2 # how many fully-connected layers after convolution and po
opt = ADAM(0.003) # optimizer

# shuffle data and pick out subset
indices = shuffle(1:size(info,1))[1:num_pts]
info = info[indices,:]
indices = shuffle(1:size(info, 1))[1:num_pts]
info = info[indices, :]
output = y[indices]

# next, make graphs and build input features (matrices of dimension (# features, # nodes))
Expand Down Expand Up @@ -74,14 +75,27 @@ train_data = zip(train_input, train_output)

# build the model
println("Building the network...")
model = Xie_model(num_features, num_conv=num_conv, atom_conv_feature_length=atom_fea_len, pool_type=pool_type, pooled_feature_length=crys_fea_len, num_hidden_layers=num_hidden_layers)
model = Xie_model(
num_features,
num_conv = num_conv,
atom_conv_feature_length = atom_fea_len,
pool_type = pool_type,
pooled_feature_length = crys_fea_len,
num_hidden_layers = num_hidden_layers,
)

# define loss function
loss(x,y) = Flux.Losses.mse(model(x), y)
loss(x, y) = Flux.Losses.mse(model(x), y)
# and a callback to see training progress
evalcb() = @show(mean(loss.(test_input, test_output)))
evalcb()

# train
println("Training!")
@epochs num_epochs Flux.train!(loss, params(model), train_data, opt, cb = Flux.throttle(evalcb, 10))
@epochs num_epochs Flux.train!(
loss,
params(model),
train_data,
opt,
cb = Flux.throttle(evalcb, 10),
)
36 changes: 20 additions & 16 deletions examples/4_sgcnn/sgcnn.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,16 @@ using Random
using ChemistryFeaturization
using AtomicGraphNets
using Serialization
<<<<<<< HEAD

#cd(@__DIR__)

graph_dir = "../../../data/OCP/traj_test_graphs/"
bulk_graph_dir = "../../../data/OCP/traj_test_bulk_graphs/"

bulk_graphs_files = readdir(bulk_graph_dir, join=true)
surf_graphs_files = readdir(graph_dir, join=true)
bulk_graphs_files = readdir(bulk_graph_dir, join = true)
surf_graphs_files = readdir(graph_dir, join = true)

# read in the graphs
=======
using CSV, DataFrames
using Statistics

Expand All @@ -44,17 +42,17 @@ info = CSV.File(csv_path) |> DataFrame
y = Array(Float32.(info[!, Symbol("energy")]))

# and the graphs
bulk_graphs_files = readdir(bulk_graph_dir, join=true)
surf_graphs_files = readdir(graph_dir, join=true)
bulk_graphs_files = readdir(bulk_graph_dir, join = true)
surf_graphs_files = readdir(graph_dir, join = true)

bulk_graphs = read_graphs_batch(bulk_graph_dir)
surf_graphs = read_graphs_batch(graph_dir)

# pick out the indices for which we have bulk graphs constructed
keep_inds = []
for i in 1:length(surf_graphs_files)
for i = 1:length(surf_graphs_files)
fn = splitpath(surf_graphs_files[i])[end]
if isfile(joinpath(bulk_graph_dir, fn)) && fn[end-3:end]==".jls"
if isfile(joinpath(bulk_graph_dir, fn)) && fn[end-3:end] == ".jls"
append!(keep_inds, [i])
end
end
Expand All @@ -64,12 +62,12 @@ y = y[keep_inds]

keep_inds = []
# now cut out any with NaN laplacians in either set
for i in 1:length(bulk_graphs)
for i = 1:length(bulk_graphs)

end

# shuffle data and pick out subset
indices = shuffle(1:size(info,1))[1:num_pts]
indices = shuffle(1:size(info, 1))[1:num_pts]
info = info[indices, :]
output = y[indices]
bulk_graphs = bulk_graphs[indices]
Expand All @@ -80,7 +78,8 @@ features = Symbol.(["Group", "Row", "Block", "Atomic mass", "Atomic radius", "X"
num_bins = [18, 9, 4, 16, 10, 10]
num_features = sum(num_bins) # we'll use this later
logspaced = [false, false, false, true, true, false]
atom_feature_vecs, featurization = make_feature_vectors(features, nbins=num_bins, logspaced=logspaced)
atom_feature_vecs, featurization =
make_feature_vectors(features, nbins = num_bins, logspaced = logspaced)

# add the features to the graphs
for ag in surf_graphs
Expand All @@ -91,7 +90,7 @@ for ag in bulk_graphs
end

# now "tuple them up"
@assert length(surf_graphs)==length(bulk_graphs) "List lengths don't match up, something has gone wrong! :("
@assert length(surf_graphs) == length(bulk_graphs) "List lengths don't match up, something has gone wrong! :("

inputs = zip(bulk_graphs, surf_graphs)

Expand All @@ -107,12 +106,17 @@ train_data = zip(train_input, train_output)

# define model, loss, etc.
model = build_SGCNN(num_features)
loss(x,y) = Flux.mse(model(x), y)
loss(x, y) = Flux.mse(model(x), y)
evalcb() = @show(mean(loss.(test_input, test_output)))
evalcb()

# train
println("Training!")
#Flux.train!(loss, params(model), train_data, opt)
@epochs num_epochs Flux.train!(loss, params(model), train_data, opt, cb = Flux.throttle(evalcb, 5))
>>>>>>> 14e39047ce8abd34f8a1a5692169371ae22397ae
@epochs num_epochs Flux.train!(
loss,
params(model),
train_data,
opt,
cb = Flux.throttle(evalcb, 5),
)
Loading