-
Notifications
You must be signed in to change notification settings - Fork 17
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
Update installation instructions #154
Changes from 17 commits
dcfaf1b
e9ff99e
ad6bb4d
4ba2057
f8d072e
bbca395
289dd38
462af6d
21e0f41
3c4e867
06f7b2c
393c5a1
d4be3b7
ec83acc
4103091
0dde201
b5a83bb
03c9a0c
36beca9
4711b17
b4f0cd3
e278182
3805e4e
4bc20c4
34ff9ac
c9ed61a
cbf811b
6a6b725
1ec4614
528c043
93694fb
75b666d
ce4da27
617e993
d2301b7
7d37a01
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,11 +77,14 @@ Installing a Development Version | |
:: | ||
|
||
pip install -r requirements.txt # Installs basic requirements | ||
pip install -r dev_requirements.txt # Installs developer requirements | ||
pip install -r docs/docs_requirements.txt # Installs documentation requirements | ||
pip install -r examples/examples_requirements.txt # Installs example requirements | ||
pip install -e . # Installs SCICO from the current directory in editable mode | ||
|
||
|
||
For installing dependencies related to the examples please see :ref:`example_notebooks`. | ||
|
||
|
||
9. The SCICO project uses the `black <https://black.readthedocs.io/en/stable/>`_, | ||
`isort <https://pypi.org/project/isort/>`_ and `pylint <https://pylint.pycqa.org/en/latest/>`_ | ||
code formatting utilities. It is important to set up a `pre-commit hook <https://pre-commit.com>`_ to | ||
|
@@ -399,3 +402,30 @@ and ``scico-data`` repositories must be updated and kept in sync. | |
:: | ||
|
||
git submodule foreach --recursive 'git push' && git push | ||
|
||
|
||
Building Documentation | ||
---------------------- | ||
|
||
A local copy of the documentation can be built from the respository root directory by doing | ||
|
||
:: | ||
|
||
python setup.py build_sphinx | ||
|
||
|
||
Alternatively: | ||
|
||
1. Navigate to the docs directory ``docs/`` | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've been changing this phrasing to "Change directory to ..." since "Navigate" seems to imply the use of a file manager, but we're referring here to the use of a terminal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also changed other occurrences of "Navigate" to "Change directory" . |
||
2. Install dependencies | ||
|
||
:: | ||
|
||
pip install -r docs_requirements.txt | ||
|
||
3. Build documentation | ||
|
||
:: | ||
|
||
make html |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,19 @@ Usage Examples | |
.. toctree:: | ||
:maxdepth: 1 | ||
|
||
.. _example_dependencies: | ||
Example Dependencies | ||
------------------------ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Underline too long? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed it |
||
|
||
Some examples may use additional dependecies. These dependencies are listed in `examples_requirements.txt <https://github.com/lanl/scico/blob/main/examples/examples_requirements.txt>`_. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "may" is redundant given the presence of the "some" - I suggest removing it. Also, there's a typo in "dependecies", and perhaps change "... dependecies. These are ..." to "... additional dependencies, which are ...". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed it. |
||
Pip should be used to install these extra requirements except astra which should be installed via conda: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest a minor rephrasing to: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks to me like this PR removes any notion of ASTRA as a dependency for SCICO. I'm uncomfortable with this because (1) ASTRA is needed to run some of the tests and (2) ASTRA-based LinOps are in the API reference without any note saying you must install ASTRA for them to work. Is there some place where we clarify (even for ourselves) what should work (examples, tests, some other thing) after the user follows which set of install instructions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, this should be handled better. See also #155. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should have a discussion sometime on issue #155. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @smajee That, or just conda, given that (I think) it's the only one needed to run the tests. See also my comment about the github action that runs the tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment in commit 617e993 |
||
|
||
:: | ||
|
||
conda install -c astra-toolbox astra-toolbox | ||
pip install -r examples/examples_requirements.txt # Installs other example requirements | ||
|
||
The dependencies can also be installed one by one as required. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps replace "one by one" with "individually"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
|
||
Organized by Application | ||
------------------------ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,56 +54,18 @@ The instructions above install a CPU-only version of SCICO. To install a version | |
|
||
:: | ||
|
||
pip install --upgrade "jaxlib==0.1.70+cuda110" -f https://storage.googleapis.com/jax-releases/jax_releases.html | ||
pip install --upgrade "jaxlib==0.1.75+cuda110" -f https://storage.googleapis.com/jax-releases/jax_releases.html | ||
|
||
|
||
|
||
Additional Dependencies | ||
----------------------- | ||
|
||
We use the `ASTRA Toolbox <https://www.astra-toolbox.com/>`_ for tomographic projectors. We currently require the development version of ASTRA, as suggested by the package maintainers. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The first sentence here seems to have been lost (rather than moved) in the edits. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed it since it seemed that we are explaining why we need astra but not why we need the other example_requirements. Shall I add a few sentences to explain how we are using each of the example_requirements or just add back this sentence on astra? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair point. I'm OK with leaving it as-is. |
||
|
||
The development version of ASTRA can be installed using conda: | ||
|
||
:: | ||
|
||
conda install -c astra-toolbox/label/dev astra-toolbox | ||
|
||
Alternatively, it can be `built from source <https://www.astra-toolbox.com/docs/install.html#for-python>`_. | ||
|
||
We also support the `Super-Voxel Model-Based Iterative Reconstruction <https://svmbir.readthedocs.io/en/latest/>`_ package as an alternative tomographic projector. Since this package can be installed via ``pip``, it is | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we mention |
||
included in the list of package dependencies (``requirements.txt``), and need | ||
not be separately installed. | ||
For installing dependencies related to the examples please see :ref:`example_notebooks`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps "For instructions on installing ..." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
|
||
|
||
For Developers | ||
-------------- | ||
|
||
For installing a version of SCICO suitable for development, | ||
see the instructions in :ref:`scico_dev_contributing`. | ||
|
||
|
||
Building Documentation | ||
---------------------- | ||
|
||
The documentation can be built from the respository root directory by doing | ||
|
||
:: | ||
|
||
python setup.py build_sphinx | ||
|
||
Alternatively: | ||
|
||
1. Navigate to the docs directory ``docs/`` | ||
|
||
2. Install dependencies | ||
|
||
:: | ||
|
||
pip install -r docs_requirements.txt | ||
|
||
3. Build documentation | ||
|
||
:: | ||
|
||
make html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not entirely "alternatively" because the docs requirements also have to be installed for the previous approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it.