+1::tada: | First off, thanks for taking the time to contribute! 🎉👍 |
---|
The following is a set of guidelines for contributing to DESC These are mostly guidelines, not rules. Use your best judgment, and feel free to propose changes to this document in a pull request.
If you just want to ask a question, the simplest method is to create an issue
on github and begin the
subject line with Question:
That way it will be seen by all developers, and
the answer will be viewable by other users.
As the user base expands and more people start using and contributing to the code, we may set up some sort of user group Slack or Discord or other forum for questions and discussion among users/developers.
Bugs are tracked as GitHub issues.
Explain the problem and include additional details to help maintainers reproduce the problem:
- Use a clear and descriptive title for the issue to identify the problem.
- Describe the exact steps which reproduce the problem in as many details as possible. When listing steps, don’t just say what you did, but explain how you did it.
- Provide specific examples to demonstrate the steps. Include links to files or copy/pasteable snippets, which you use in those examples. If you’re providing snippets in the issue, use Markdown code blocks.
- Describe the behavior you observed after following the steps and point out what exactly is the problem with that behavior.
- Explain which behavior you expected to see instead and why.
- Include plots of results that you believe to be wrong.
- If you’re reporting that DESC crashed, include the python stack trace and full error message. Include the stack trace in the issue in a code block, a file attachment, or put it in a gist and provide link to that gist.
Provide more context by answering these questions:
- Did the problem start happening recently (e.g. after updating to a new version of DESC) or was this always a problem?
- If the problem started happening recently, can you reproduce the problem in an older version of DESC? What’s the most recent version in which the problem doesn’t happen?
- Can you reliably reproduce the issue? If not, provide details about how often the problem happens and under which conditions it normally happens.
Include details about your configuration and environment:
- Which version of DESC are you using?
- Which version of JAX (and other dependencies) are you using
- What’s the name and version of the OS you’re using?
- Are you running DESC in a virtual machine? If so, which VM software are you using and which operating systems and versions are used for the host and the guest?
- Are you running DESC locally or on a cluster? If so, which cluster, with which modules loaded?
- What hardware are you running on? Which CPU, GPU, RAM, etc. If on a cluster, what resources are you allocating?
This section guides you through submitting an enhancement suggestion for DESC, including completely new features and minor improvements to existing functionality.
Before creating enhancement suggestions, please check this list as you might find out that you don’t need to create one. When you are creating an enhancement suggestion, please include as many details as possible, including the steps that you imagine you would take if the feature you’re requesting existed.
- Check the documentation for tips — you might discover that the enhancement is already available.
- Perform a cursory search to see if the enhancement has already been suggested. If it has, add a comment to the existing issue instead of opening a new one.
Enhancement suggestions are tracked as GitHub issues. After you’ve followed the above steps and verified that an issue does not already exist, create an issue and provide the following information:
- Use a clear and descriptive title for the issue to identify the suggestion.
- Provide a step-by-step description of the suggested enhancement in as many details as possible.
- Provide specific examples to demonstrate the steps. Include copy/pasteable snippets which you use in those examples, as Markdown code blocks.
- Describe the current behavior and explain which behavior you expected to see instead and why.
- Explain why this enhancement would be useful to other users.
- Provide references (if relevant) to papers that discuss the physics behind the enhancement, or describe the enhancement in some way.
Unsure where to begin contributing to DESC? You can start by looking
through these good first issue
and help wanted
issues:
- Good first issues - issues which should only require a few lines of code, and a test or two.
- Help wanted issues - issues which should be a bit more involved than beginner issues.
Once you've made your changes on a local branch, open a pull request on github. In the description, give a summary of what is being changed and why. Try to keep pull requests small and atomic, with each PR focused on a adding or fixing a single thing. Large PRs will generally take much longer to review and approve.
Opening a PR will trigger a suite of tests and style/formatting checks that must pass before new code can be merged. We also require approval from at least one (ideally multiple) of the main DESC developers, who may have suggested changes or edits to your PR.
What if the test_compute_everything
test fails, or there is a conflict in master_compute_data_rpz.pkl
?
When the outputs of the compute quantities tested by the test_compute_everything [test](https://github.com/PlasmaControl/DESC/blob/master/tests/test_compute_everything.py) are changed in a PR, that test will fail. The three main reasons this could occur are:
- The PR was not intended to change how things are computed, but messed up something unexpected and now the compute quantities are incorrect, if you did not expect these changes in the PR then look into why these differences are happening and fix the PR.
- The PR updated the way one of the existing compute index quantities are computed (either by a redefinition or perhaps fixing an error present in
master
) - The PR added a new class parametrization (such as a new subclass of
Curve
likeLinearCurve
etc)
If the 2nd case is the reason, then you must update the master_compute_data_rpz.pkl
file with the correct quantities being computed by your PR:
- First, run the test with
pytest tests -k test_compute_everything
and inspect the compute quantities whose values are in error, to ensure that only the quantities you expect to be different are shown (and that the new values are indeed the correct ones, you should have a test elsewhere for that though). - If the values are as expected and only the expected compute quantities are different, then replace the block
if not error_rpz and update_master_data_rpz:
# then update the master compute data
with
if True or (not error_rpz and update_master_data_rpz):
# then update the master compute data
- rerun the test
pytest tests -k test_compute_everything
, now any compute quantity that is different between the PR and master will be updated with the PR value git restore tests/test_compute_everything.py
to remove the change you made to the testgit add tests/inputs/master_compute_data_rpz.pkl
and commit to commit the new data file
If the 3rd case is the reason, then you must simply add the new parametrization to the test_compute_everything
[test](https://github.com/PlasmaControl/DESC/blob/master/tests/test_compute_everything.py)
things
dictionary with a sensible example instance of the class to use for the test, and- to the
grid
dictionary with a sensible default grid to use when computing the compute quantities for the new class - Then, rerunning the test
pytest tests -k test_compute_everything
will add the compute quantities for the new class and save them to the.pkl
file git add tests/inputs/master_compute_data_rpz.pkl
and commit to commit the new data file
- Follow the PEP8 format where possible
- Format code using black before committing - with formatting, consistency is better than "correctness." We use version
22.10.0
(there are small differences between versions). Install withpip install "black==22.10.0"
. - Check code with
flake8
, settings are insetup.cfg
- We recommend installing
pre-commit
withpip install pre-commit
and then runningpre-commit install
from the root of the repository. This will automatically run a number of checks every time you commit new code, reducing the likelihood of committing bad code. - Use Numpy Style Docstrings - see the code for plenty of examples. At a minimum, the docstring should include a description of inputs and outputs, and a short description of what the function or method does. Code snippets showing example usage strongly encouraged.
- Readability and usability are more important than speed 99% of the time.
- If it takes more than 30 seconds to understand what a line or block of code is doing, include a comment summarizing what it does.
- If a function has more than ~5 inputs and/or return values, consider packaging them in a dictionary or custom class.
- Make things modular. Focus on small functions that do one thing and do it well, and then combine them together. Don’t try to shove everything into a single function.
- It’s not Fortran! You are not limited to 6 character variable
names. Please no variables or functions like
ma00ab
orpsifac
. Make names descriptive and clear. If the name and meaning of a variable is not immediately apparent, the name is probably wrong. - Sometimes, a shorter, less descriptive name may make the code more
readable. If you want to use an abbreviation or shorthand, include a
comment with the keyword
notation:
explaining the notation at the beginning of the function or method explaining it, eg# notation: v = vartheta, straight field line poloidal angle in radians
.
DESC makes heavy use of the JAX library for accelerating code through
JIT compiling and automatic differentiation. JAX has a submodule,
jax.numpy
, commonly abbreviated as jnp
which offers an API
almost identical to numpy
.
- If the function will ever be used for optimization (i.e., called as
part of an objective function), use
jnp
. - Similarly, if the function will need to be called multiple times and
could benefit from JIT compiling, use
jnp
andjit
. However, in general it is best to onlyjit
the outermost function, not each subfunction individually. - If the function will ever need to be differentiated through, use
jnp
andjacfwd
,jacrev
, orgrad
. - If you are certain it will only ever be used during initialization or
post processing (i.e. plotting), feel free to use
np
, as it can be slightly faster without JIT compilation, and has fewer tricks necessary to make it work as expected. - If in doubt,
jnp
is usually a safe bet. jax.numpy
is almost a drop in replacement fornumpy
, but there are some subtle and important differences.
The testing suite in DESC is based on pytest, and makes use of several plugins for specialized testing. You can install all the necessary tools with pip install -r devtools/dev-requirements.txt
. You can run the tests from the root of the repository with pytest -m unit
for unit tests or pytest -m regression
for full regression tests (unit tests should take ~1hr on a standard laptop, regression tests may take several hours). To only run selected tests you can use pytest -k foo
which will only run tests that have foo
in the test or file name.
Note: when adding new tests to DESC, they must either be marked with @pytest.mark.unit
or @pytest.mark.regression
, otherwise they will not be run as part of the automatic CI testing.
Additional useful flags include:
--mpl
tells pytest to also compare the output of plotting functions with saved baseline images intests/baseline/
using pytest-mpl. These baseline images can be regenerated withpytest -k plotting --mpl-generate-path=tests/baseline/
.--cov
will tell it to also report how much of the code is covered by tests using pytest-cov. A summary of the coverage is printed to the terminal at the end of the tests, and detailed information is saved to a.coverage
file, which can then be turned into a simple HTML page withcoverage html
. This will create ahtmlcov/
directory in the root of the repository that can be viewed in a browser to see line by line coverage.
- A commit message template is included in the repository,
.gitmessagetemplate
- You can set the template to be the default with
git config commit.template .gitmessagetemplate
Some helpful rules to follow (also included in the template):
- Separate subject line from body with a single blank line.
- Limit the subject line to 50 characters or less, and wrap body lines at 72 characters.
- Capitalize the subject line.
- Use the present tense (“Add feature” not “Added feature”) and the imperative mood (“Fix issue…” not “Fixes issue…”) in the subject line.
- Reference issues and pull requests liberally in the body, including
specific issue numbers. If the commit resolves an issue, note that at
the bottom like
Resolves: #123
. - Explain what and why vs how. Leave implementation details in the code. The commit message should be about what was changed and why.
- Use SphinxDoc.
- Use Numpy Style Docstrings.
- Use reStructuredText.