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 and add FAQ. Handle warnings #146

Merged
merged 15 commits into from Nov 23, 2017

Conversation

Projects
None yet
2 participants
@bocklund
Collaborator

bocklund commented Nov 12, 2017

Fixes gh-126, fixes gh-128, fixes gh-134, and fixes gh-139

Regarding points in #126:

  • Update installation to first suggest anaconda installation for all platforms. Also update the development mode, because conda-build is not well maintained for using development mode in environments. Suggested to use pip or setup.py in conda/conda-build#1992

This PR updates the installation guide with step by step instructions for all platforms, but emphasizing Anaconda. It is now more clear that you can use either distribution to install pycalphad.

The installation includes the caveats of the Ipopt installation situation for pip installation and develop mode, as well as a Anaconda alternative to develop mode.

  • Add page describing parallelism with dask

This is in the newly added FAQ

  • Estimate of computation time in the examples (roughly ~1 minute accuracy)

A brief statement of computation time and the implications for binplot/ternplot were added to the FAQ.

  • (Possible) mention which warnings are normal to see, including convergence failures. Where should this go?

I did not handle the convergence failures (Ipopt should make them more rare). This commit explicitly adds code to filter out the NumPy warnings and fixes the dask import warning that users have been seeing and bumps the dask version accordingly. (Python 3.7 will make async a keyword (see https://bugs.python.org/issue30406). This means dask had to rename their async module to local to avoid having a module name be a keyword and users have been seeing warnings about the import. This commit updates the import from dask.async.get_sync to dask.local.get_sync and bumps the requirements to dask>=0.15.).

#139: Addressed simultaneously with #126

#128: added to FAQ

#134: Added a UsingCalculationResults example notebook (and generated docs) that goes through the calculate and equilibrium datasets and describes them fully. Also provides some brief examples of how one would select and mask using xarray.

@bocklund bocklund requested a review from richardotis Nov 12, 2017

@bocklund

This comment has been minimized.

Collaborator

bocklund commented Nov 12, 2017

There were some minor code changes, but mostly docs here. I suggest checking out this branch locally and cd docs, then building the docs and inspecting them with something like sphinx-build . _build/html && cd _build/html && python3 -m http.server && cd ../..

----------------------------------------------------------
This is an upstream bug in sympy, where floats are unable to be pickled.
The fix has been copmleted, but not yet released. While the fix is not released,

This comment has been minimized.

@richardotis

richardotis Nov 12, 2017

Collaborator

spelling

bocklund added some commits Sep 27, 2017

@bocklund bocklund force-pushed the install-docs branch from 5dcd5ba to b47de13 Nov 12, 2017

@richardotis richardotis added this to the 0.6 milestone Nov 13, 2017

@richardotis

This comment has been minimized.

Collaborator

richardotis commented Nov 13, 2017

  • Al-Fe phase diagram picture on the index page is a broken link. (It's been that way a while.)
  • Anaconda is a scientific Python distribution by Continuum Analytics. They call themselves Anaconda, Inc. now.
  • This can be fixed in the FAQ needs a trailing colon.
  • All the examples need new titles; shorter would be better. "Calculation" is in almost every one and doesn't help the user very much. (Almost every feature of pycalphad involves calculation.)
  • Using Calculation Results: Headings should be calculate() Results and equilibrium() Results to be unambiguous
    -- Masking (mentioned at bottom) needs an example
    -- It would also be nice to mention .values for conversion to a NumPy array

@richardotis richardotis self-assigned this Nov 13, 2017

@bocklund

This comment has been minimized.

Collaborator

bocklund commented Nov 13, 2017

I think with the state of the solver, Al-Ni is a better looking example to show rather than Al-Fe. As far as the masking, the where is what that is: everything that does not match the condition is nan.

@richardotis richardotis assigned bocklund and unassigned richardotis Nov 14, 2017

@richardotis

This comment has been minimized.

Collaborator

richardotis commented Nov 14, 2017

Either phase diagram is fine. We just need to make sure the link on the front page is correct.

@bocklund

This comment has been minimized.

Collaborator

bocklund commented Nov 14, 2017

TODO: Add notes on units in the FAQ

@bocklund

This comment has been minimized.

Collaborator

bocklund commented Nov 16, 2017

This is ready for another look

@richardotis

This comment has been minimized.

Collaborator

richardotis commented Nov 23, 2017

Merge when ready.

@bocklund bocklund merged commit 75982c1 into develop Nov 23, 2017

5 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 87.733%
Details

@bocklund bocklund deleted the install-docs branch Nov 25, 2017

richardotis added a commit that referenced this pull request Nov 26, 2017

Update installation and add FAQ. Handle warnings (#146)
* WIP: #126 install docs

* DOC: Update installation instructions

* DOC: Add FAQ and address parallelism

* DOC: Add notes about equilibrium computation time to FAQ

* DOC: Add argument is not an mpz to FAQ

* ENH: Filter NumPy warnings for dividing by zero and invalid values in true divide

* MAINT: Silence dask async deprication warning and bump requirements

Python 3.7 will make async a keyword
(see https://bugs.python.org/issue30406). This means dask had to rename
their async module to local to avoid having a module name be a keyword
and users have been seeing warnings about the import. This commit updates
the import from dask.async.get_sync to dask.local.get_sync and bumps the
requirements to dask>=0.15.

* DOC: Add matplotlib text cut off fix to FAQ

* DOC: Add using calculation results example for xarray Datasets

* DOC: Fix several spelling errors

* DOC: Update examples titles

* DOC: Regenerate API docs for removed modules

* DOC: Fix homepage image (AL-NI), minor spelling corrections

* DOC: Update UsingCalculationResults to use .values (NumPy)

* DOC: Add units to FAQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment