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

Update documentation #210

Merged
merged 12 commits into from Mar 23, 2022
Merged

Update documentation #210

merged 12 commits into from Mar 23, 2022

Conversation

phoebe-p
Copy link
Member

@phoebe-p phoebe-p commented Mar 17, 2022

Addresses some issues with the documentation:

  • There was no explanation of when S4 needs to be installed on the main installation page, only on the 'Windows 10' page with special instructions. This is confusing. I made a page which links to my S4 fork, where I maintain up-to-date installation instructions for both MacOS and Ubuntu in the README. This page is linked under the bullet point discussing Fortran compilers for the PDD solver.
  • The Windows 10 page now contains caveats about the Windows subsytem for Linux. In general I find that if people are inexperienced enough to be following these instructions (and don't already have an Ubuntu virtual machine/dual boot), they will probably not be happy using the command line for everything and it is in fact very annoying for many of the common uses of Solcore since you have to save every plot and then navigate to it manually to look at it. I think for most users a virtual machine or dual boot is a better option so I have noted this.
  • Fixed some general formatting issues, typos etc.
  • For installation in development mode, change .[dev] to ".[dev]" for the reasons discussed here.

@codecov
Copy link

codecov bot commented Mar 17, 2022

Codecov Report

Merging #210 (502e635) into develop (883a31a) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff            @@
##           develop     #210   +/-   ##
========================================
  Coverage    45.84%   45.84%           
========================================
  Files           84       84           
  Lines         9091     9091           
========================================
  Hits          4168     4168           
  Misses        4923     4923           
Impacted Files Coverage Δ
solcore/light_source/smarts.py 11.45% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 883a31a...502e635. Read the comment docs.

@phoebe-p phoebe-p marked this pull request as ready for review March 21, 2022 03:43
@phoebe-p phoebe-p requested a review from dalonsoa March 21, 2022 03:43
@phoebe-p
Copy link
Member Author

Ok, some more changes:

  • added .DS_Store file to gitignore
  • edited the custom materials example in the documentation. It still had the lines for manually adding entries to the config file which is no longer necessary. Example is now the same as the one in the examples folder.
  • Fixed import of RCWASolverError for optics method comparison example in the documentation
  • Removed the note about thick layers in tutorial.rst -- we can put this back but with the implementation of incoherent calculations (and their default use for thick layers) I feel like this might be confusing
  • Removed duplicated code block in tutorial.rst
  • Added a note about the known issue (PDD installation fails on Apple M1 architecture #209) about installing the PDD on new Apple computers on the documentation page about getting a suitable Fortran compiler. It is possible to do this but requires some extra steps
  • Added an entry to the list of common issues about potential problems when calling the PDD solver when the solcore directory is the working directory (I don't know what the cause of the problem is, just that it can cause issues).
  • Other minor updates, typo fixes etc. in the documentation and examples.

@phoebe-p
Copy link
Member Author

Some of the tests are failing, but I think this is just for the same reason that the daily tests are sporadically failing. We should try to address this at some point but I don't know what's causing it, because it does not happen every time and the error is not even the same each time....

Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

I've made a few suggestions, but it looks good otherwise. After merging this one, I suggest to open a PR against main, review, merge and create a new release.

docs/source/Examples/example_3J_with_DA_solver.rst Outdated Show resolved Hide resolved
docs/source/Installation/Solcore_on_Windows.md Outdated Show resolved Hide resolved
docs/source/Installation/compilation.md Show resolved Hide resolved
docs/source/Installation/installation.rst Outdated Show resolved Hide resolved
docs/source/Installation/installation.rst Outdated Show resolved Hide resolved
@phoebe-p phoebe-p merged commit 29593db into develop Mar 23, 2022
@phoebe-p phoebe-p deleted the examples_docs branch February 13, 2023 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants