MHKiT-Software / MHKiT

MHKiT Documentation
https://MHKiT-Software.github.io/MHKiT/
4 stars 14 forks source link

Update to include IO NDBC and environmental contours example. #33

Closed ssolson closed 4 years ago

ssolson commented 4 years ago

Here I have updated the docs to include some changes from the WDRT integration. Primarily updating language about spectral data and making sure the environmental contours example is up.

Please work from this branch as we update for the loads PR and WEC-SIM IO PR

kmruehl commented 4 years ago

@ssolson this looks good. I made some minor revisions, and I think we should go ahead and merge this PR. We can add in the remaining documentation when we're ready to tag v0.3.0.

ssolson commented 4 years ago

Thanks Rebecca. I was installing MHKiT-MATLAB for my review and I had some trouble. I thought we/ I could address it in this PR. I need your feedback to make sure I am interpreting the instructions correctly before I change anything.

As shown in the screen shot of the installation instructions below I saw that pyversion returned the correct version of my python installation. The instructions then state: *"If the resulting Python version is 3.6 or 3.7, skip to the Test the Installation section" (We also build Python 3.8 so this should be updated). This would skip the mhkit_python_utils package section which I believe you must do? So is this statement correct?

I thought I should not skip this so I followed the instructions in the mhkit_python_utils package section, where the user is instructed to run a pip install -e . (I think we should clarify in the command window?)

After doing this I was at the Test installation section where I ran the test and I got the following error

>> [x,y]=circular(30)
Unrecognized function or variable 'circular'.

So then I doubled clicked on the mhkit.mltbx in the MATLAB GUI which install MHKiT as a toolbox. Should this be part of the installation?

After doing this I got a new error from the test:

[x,y]=circular(30)
Unable to resolve the name py.mhkit.river.performance.circular.

So then I added mhkit_python_utils folder and mhkit folder & subfolders to finally get the correct result. Is adding these to the path part of the installation?

image

rpauly18 commented 4 years ago

@ssolson Thank you for pointing out that changes that need to happen in the installation Docs text. Yes, if the user is pointing to the correct version of Python, they should go to "MHKiT_python_utils" section. We should also clarify the location of doing the install. It is in the Docs to double click on the toolbox in the first block of text in the MHKiT-MATLAB installation, but perhaps we should make it stand out more.

You should not need to add the folders to your path for MHKiT-MATLAB to work as long as you have the corresponding version of MHKiT-Python on your machine. Are you certain that the version of MHKiT-Python that you were working with, or had "checkout out" had the most up to date organization of the river module?

ssolson commented 4 years ago

So I have Kelly's feature_wecsim as my mhkit python version when running this. That was a mistake obviously. This fork does not have the river device renamed to performance. Let me see if I can try again with the MHKiT master branch

ssolson commented 4 years ago

This fixed the issue. Thank you.

kmruehl commented 4 years ago

@ssolson and @rpauly18 is this ready for a merge?

I think that the cleanest way to manage the documentation is for us to update the documentation after each new feature is merged into mater (instead of trying to do it all at once). In short, once a new feature is merged into master, we can update the documentation submodules, recompile the docs, and make changes as needed.

rpauly18 commented 4 years ago

@kmruehl The only drawback to continually updating the Docs is that these new functions and the new breakdown are not available via a pip install yet. There is the possibility that a user could get frustrated/ confused if a function they expect to be there is not in the package they pip install.

ssolson commented 4 years ago

No I have not pushed changes for the MATLAB installation.

I think Rebecca makes a really good point. I know there is a way to have both where the docs point to a stable release or the master/ current...but not it on implementing it! :)

ssolson commented 4 years ago

Reviewing my build the terminology symbols are not building correctly. I have the specified verion of the packages listed on the documentation repo: Sphinx --> Version: 3.1.1 nbsphinx --> Version: 0.7.1 sphinxcontrib-matlabdomain --> Version: 0.11.2 sphinxcontrib-bibtex --> Version: 1.0.0 sphinx-rtd-theme --> Version: 0.5.0

Do you guys know what is wrong?

image

kmruehl commented 4 years ago

@ssolson I do not have this issue with the compiled docs, but it should be related to sphinx.ext.imgmath since that's the package we use that converts all math text to images.

rpauly18 commented 4 years ago

@ssolson this is the same issue I have had for a while now when compiling the docs and have not been able to figure out how to fix it yet.

kmruehl commented 4 years ago

@rpauly18 and @ssolson I switched the math to sphinx.ext.mathjax and that should resolve your issue. sphinx.ext.imgmath is what I used before and it requires a LaTeX compiler. Let me know if you still have this issue now.