search

SNL-WaterPower / WecOptTool-MATLAB

WEC Design Optimization Toolbox
GNU General Public License v3.0
12 stars 9 forks source link

Alpha Testing #65

Closed H0R5E closed 4 years ago

H0R5E commented 4 years ago

A list of things to do prior and during alpha testing:

ryancoe commented 4 years ago

I'd really like to get this to alpha testers by COB today. Please either wrap up the outstanding items or consider them unnecessary (e.g., I'm ok with people cloning the repo).

H0R5E commented 4 years ago

Yeah, I agree. I'm going to merge #68 now and then I will put together a zip file of the code and docs and put it somewhere. The alpha testers can then work from this zip.

ssolson commented 4 years ago

Ryan could you review my comments in the ghPages #63 review while Mat builds the release?

H0R5E commented 4 years ago

Here you go:

wecopttool_v01_rc1.zip

ssolson commented 4 years ago

I have approved the #63 we should update the docs to reflect the changes. I am running the test suite now. Documentation works.

ssolson commented 4 years ago

I had 2 tests fail. I am looking into the call to strsplit() in the tests which run example.m and dependencyCheck.m which I ran independent runTests.m no problem.

test_results.pdf

ssolson commented 4 years ago

This seems to work if I define what the string is first. Not sure what the issue is here. Any suggestions?

K>> strsplit(s.path, '\')
Error using strsplit
No value was given for '\'. Name-value pair arguments require a name followed by a value.

Error in strsplit (line 99)
    p.parse(varargin{:});

K>> str = s.path;
K>> strsplit(str, '\')

ans =

  1×8 cell array

  Columns 1 through 7

    {'C:'}    {'Users'}    {'sterl_000'}    {'Desktop'}    {'alphaWOT'}    {'wecoopttool'}    {'toolbox'}

  Column 8

    {'+WecOptLib'}
ssolson commented 4 years ago

Ahh! Its finding both +WecOptLibs. We should flag this as an issue but presumably, users will not have multiple WecOptTool Installations?


K>> s.path

ans =

    'C:\Users\sterl_000\Desktop\alphaWOT\wecoopttool\toolbox\+WecOptLib'

ans =

    'C:\Users\sterl_000\Documents\WecOptTool\toolbox\+WecOptLib'
H0R5E commented 4 years ago

I think this is what being temperamental again. I have a way of getting this path more robustly now, so I'll update it now.

H0R5E commented 4 years ago

OK, that update is in #73. I'll assume it's OK and put together a new RC.

H0R5E commented 4 years ago

RC2!

wecopttool_v01_rc2.zip

ryancoe commented 4 years ago

@kmruehl @zmorrell-sand @dforbush2 - You've all been asked to perform alpha testing reviews of WecOptTool. We realize this is short notice and a short timeline, so we'll be happy to with whatever you're able to provide by COB Friday. Here's some instructions/notes:

To get started, download the release candidate from this thread. This .zip file contains the source code (i.e., the git master) and documentation.

Answering some questions you've had so far:

Thanks for your help.

H0R5E commented 4 years ago
zmorrell-sand commented 4 years ago

I will try setting up and running this on both Windows and Linux to make sure that it is compatible across platforms. I will let you know if I have any questions or find anything weird

zmorrell-sand commented 4 years ago

One minor thing I noticed that may be a good idea to address, the example.m file is dependent on has "use-parallel" option set to True, but this requires the Parallel Computing Toolbox if I am not mistaken. I am unsure of whether MATLAB will default to the non-parallel case if the user does not have the toolbox. We specify in the README and html documentation that the Parallel Optimization Toolbox is optional. I looked into it a little bit, and it seems that license('test', 'Distrib_Computing_Toolbox') can test if someone has access to the toolbox, so perhaps we should place the options inside of an if statement depending on this command?

zmorrell-sand commented 4 years ago

Everything seems to run well on Windows. I made sure to follow the installation instructions directly and ignore any prior knowledge I had about the program. Installation went smoothly and worked out of the box. I made sure to run all the test cases and example scripts, and it all worked well.

Now onto linux.

kmruehl commented 4 years ago

Okay, I'm going to provide my feedback in a few comments. This is what I have thus far.

Index

Installation

zmorrell-sand commented 4 years ago

As Kelley mentioned, addpath(genpath('/path/to/wecopttool/')) needs to be used on Linux, or installNemoh will not run properly. Other than that, everything runs well out of the box on Linux as well

dforbush2 commented 4 years ago

I was able to install and run on windows, Matlab 2019a. However, my run of example had "Solver Stopped Prematurely" warnings for fmincon.

Code is mostly clear. Its like Matlab wearing a python costume.

WEC-Sim can conflict with WAFO if they are both on the path at the same time, and since you might share some of the user base, I personally like having to manually add WAFO to my path here rather than a startup file, but I could see the benefits either way.

I think for a version 0.1 release you guys look good!

It would be great to see the example documentation fleshed out a little more...at least labeling the output figure (I think its power for each of the 8 query sea-states?).

Thinking longer term, it would be great to see a more populated theory/workflow section to get a better idea of whats under the hood and what user I/Os are available. I'm a little concerned about how extendable this meshing approach will be to non-axisymmetric geometries, as Nemoh leaves some to be desired on this front. If you intend to actively support this code I worry you'll have the same issues we do with WEC-Sim where half of our posted issues are actually Nemoh issues.

H0R5E commented 4 years ago

Thanks everyone for your feedback. My fault about the loss of genpath. Casual change because it wasn't needed in Windows. I'll try and address all your notes over the weekend. Thanks again. T

kmruehl commented 4 years ago

Okay, and here's my second round of comments

Example

Running the Example

Feedback_kmr_example

API Doc

Tests

General

kmruehl commented 4 years ago

Thanks everyone for your feedback. My fault about the loss of genpath. Casual change because it wasn't needed in Windows. I'll try and address all your notes over the weekend. Thanks again. T

@H0R5E I'm running windows and it was required for me.

H0R5E commented 4 years ago

Ok, thanks. I guess its also a backwards compatibility thing then. For my version it just picks up package directories automatically. Ill pop it back in as it wont do any harm. We should definitely make a note as to the oldest version we know to be operable.

kmruehl commented 4 years ago

I was able to install and run on windows, Matlab 2019a. However, my run of example had "Solver Stopped Prematurely" warnings for fmincon.

I was going to recommend against using fmincon because we've had issues with it before as well. I don't remember what the issue was, but I know it's a problematic function.

kmruehl commented 4 years ago

@H0R5E unfortunately MATLAB changes things like that all the time. It's very frustrating, and we have to do version capability checks for MATLAB changes all the time. Recently, for example, array output was changed b/w rows and columns so that messed up all of our matrix calculations. Specifying version compatibility is critical.

I'm running MATLAB 2018b on Windows and I have Optimization Toolbox Version 8.2 (R2018b)

kmruehl commented 4 years ago

WEC-Sim can conflict with WAFO if they are both on the path at the same time, and since you might share some of the user base, I personally like having to manually add WAFO to my path here rather than a startup file, but I could see the benefits either way.

@dforbush2 did you mean WecOptTool instead of WAFO? I didn't have this issue and I don't think there should be a path issue by adding the WecOptTool path to startup.m. Plus, if people are already using WEC-Sim, adding another path to the startup.m should be a simple addition.

kmruehl commented 4 years ago

It would be great to see the example documentation fleshed out a little more...at least labeling the output figure (I think its power for each of the 8 query sea-states?).

Thinking longer term, it would be great to see a more populated theory/workflow section to get a better idea of whats under the hood and what user I/Os are available. I'm a little concerned about how extendable this meshing approach will be to non-axisymmetric geometries, as Nemoh leaves some to be desired on this front. If you intend to actively support this code I worry you'll have the same issues we do with WEC-Sim where half of our posted issues are actually Nemoh issues.

I wholeheartedly agree with this feedback.

kmruehl commented 4 years ago

okay, I'm out for the weekend. @H0R5E have fun with all of this, sorry if I gave you more than you were asking for :)

H0R5E commented 4 years ago

One minor thing I noticed that may be a good idea to address, the example.m file is dependent on has "use-parallel" option set to True, but this requires the Parallel Computing Toolbox if I am not mistaken. I am unsure of whether MATLAB will default to the non-parallel case if the user does not have the toolbox. We specify in the README and html documentation that the Parallel Optimization Toolbox is optional. I looked into it a little bit, and it seems that license('test', 'Distrib_Computing_Toolbox') can test if someone has access to the toolbox, so perhaps we should place the options inside of an if statement depending on this command?

@zmorrell-sand, thanks this is a really good point. I've opened this one in #83.

As Kelley mentioned, addpath(genpath('/path/to/wecopttool/')) needs to be used on Linux, or installNemoh will not run properly. Other than that, everything runs well out of the box on Linux as well

@zmorrell-sand and @kmruehl thanks, I've got this one TODO in #84 now.

H0R5E commented 4 years ago

@kmruehl, I'm going to reply inline

Okay, I'm going to provide my feedback in a few comments. This is what I have thus far.

Index

  • Add a more detailed description about what the WecOptTool is and what it's used for. Why do people want to use this tool? What can it do for them? Why should they bother installing/using this toolbox? A user may think it looks like a lot of work to install, and they don't know why they would want to use it.

Yeah, I think we need this.

  • Contents/User Guide headers are redundant

Agreed, I have this structure to allow for multiple major sections (like User vs Technical) but it's not needed right now.

Installation

  • "Stable or development versions are available." Where is development, in a dev branch? On a fork? Will development be on master, so the "stable" version is the tagged release? How do I contribute do its development?

Contribution instructions are in the README, and we should add a note pointing people there for interested parties

  • I recommend using the .. Note syntax for notes
  • Specify version of the dependency, will the MATLAB Optimization Toolbox from 2015b work? I recommend specifying the version you've run as a minimum requirement

Yep, this is important.

  • The first step to remove an existing installation is confusing. I recommend making that a note. First step should be download the code via a clone or download.

OK, we'll take another look at this, thanks. Bad things happen if you install two versions.

  • For WEC-Sim we have a 'wecsimstartup.m' file that adds the path, see documentation here, you may consider having a startup file in the distribution with a relative path to add the path for the toolbox. You could also check for dependencies. Path issues are one the biggest issues we see, so the easier you can make it on users, the better.

Personally, I don't think startup files are any more reliable given the user may wish to change the startup directory depending on what they are working on at any one time. If there were a single unified location where these could be placed then that would be more useful, but the user still has to do some stuff. Our plan is take the next step in functionality with #58.

  • Should be addpath(genpath('C:\Users\kmruehl\Documents\GitHub\WecOptTool\wecopttool_v01_rc3\wecopttool')), otherwise subfolders are not added to directory.

See #84.

  • The savepath command gave me a weird warning. May consider having users create a 'startup.m' file in their MATLAB path that gets executed every time MATLAB is opened instead of saving the path

What did the warning say? As I said, not sure I see the relative value of a startup file.

  • update documentation to specify correct NEMOH bin file directory, it's C:\Users\kmruehl\Documents\GitHub\Nemoh\Release, not Nemoh\bin

The documentation does give specific Windows vs Linux instructions, but we can try to make them more distinct by rolling them up.

  • Until I added the genpath, I got this error, Undefined variable "WecOptLib" or class "WecOptLib.utils.getUserPath". when trying to install nemoh. I recommend having an output of the 'installNemoh' function that tells users the install was successful.
  • I like the dependencyCheck functionality, but I think it should be added to the end of the NEMOH install function and run automatically.

This is the subject of #69. I'll mark it high priority.

  • I like the runTests functionality, and the command window output it nice, but the documentation should also tell me what a successful 'runtests' looks like. Looks like I got a successful run Totals: 27 Passed, 0 Failed, 0 Incomplete.

Agreed. Thanks.

Copyright/License

  • Why was GNU open-source license selected? This will likely be limiting to developers who want to customize the toolbox for their application.

See the discussion in #60

  • Where is the Copyright assertion in the LICENSE file? <program> Copyright (C) <year> <name of author> This program comes with ABSOLUTELY NO WARRANTY; for details type 'show w'. This is free software, and you are welcome to redistribute it under certain conditions; type 'show c' for details. <one line to give the program's name and a brief idea of what it does.> Copyright (C) <year> <name of author>

I'm not sure you need to add this to the LICENSE file for GPL as every source file carries it and references to the text in licence file. The README should carry it though.

This should go through Sandia legal and have an NTESS assertion.

@ryancoe has this been done?

  • Copyright assertion also needs to be added to documentation: /wecopttool_v01_rc3/html_docs/user/license.html

I think the verbatim licence file should be replaced with the source code boilerplate.

  • Copyright assertion in source code should not be to "Sandia National Labs". This is what the assertion should look like: Copyright 2014 National Technology & Engineering Solutions of Sandia, LLC (NTESS). Under the terms of Contract DE-NA0003525 with NTESS, the U.S. Government retains certain rights in this software.

@ryancoe can you check this? We'll update it (in all the files - urg) if we have to.

Thanks again Kelley. I've summarised these in #85 and #86.

H0R5E commented 4 years ago

Okay, and here's my second round of comments

Right, I've put all your notes regarding content in the example into #87. Many thanks. I'll just address some of the other operational things you posted.

Example

  • My MATLAB Workspace has a lot of variables in it. Is there an output object? If so, which one is it? Consider clearing all internal toolbox variables, and only keeping the inputs/ouputs in the workspace upon completed run

I think this is a drawback of having example.m as a script (but this is the most likely way it will be used) and I don't think it should be outputting anything that isn't in example.m. Are you seeing any variables being created that are not defined in example.m?

  • Why not use MHKiT instead of WAFO? This is also developed through WPTO funding and we can modify it as need be to support this program. It would be nice to see WPTO funded projects leverage other WPTO funded projects.

Good suggestion! Opened in #88.

  • The plot shows up as Figure 29, even though I have no other figures open.

Yeah, I've seen this too. Needs investigating. Opened in #89.

API Doc

  • We had a lots of issues implementing the API documentation for MHKiT-MATLAB when we had the source code on master and the documentation on gh-pages. I'm curious how you handled that.

So the plan is to store the sphinx source on master and push the build to gh-pages. You can follow #82 for the implementation until its merged. The next step is to automatically build the docs on a master branch merge. See #72 and #81.

Tests

  • How are you running your unit tests? Do you have a runner on Jenkin? @zmorrell-sand just set up a Jenkins server with MATLAB to run WEC-Sim unit tests if you want to do the same thing for this tool

We are just running them manually at the moment and posting the run_tests.pdf file to our PRs for validation. It would be nice to get this running automagically, for sure.

General

  • I have a lot of questions about the overall theory/application of the code. It seems like it could be a very useful tool once a general geometry has been decided upon, and someone wants to refine its general dimension. Is that the intended use of this tool? This should be explained up front and be very clear. Users need to know why they would want to use this, and what it can accomplish.

Yeah, I think there is a balance of enticing without over-selling. This is just a beta, so we don't want to promise too much and not deliver otherwise people might not come back [ to DTOcean :'( ]. @ryancoe what's your thoughts here?

H0R5E commented 4 years ago

@dforbush2, thanks for your feedback. I'll reply inline:

I was able to install and run on windows, Matlab 2019a. However, my run of example had "Solver Stopped Prematurely" warnings for fmincon.

Yes, we deliberately stop the optimiser so that the example runs quicker. We need to make a note about this, so I've opened #90.

Code is mostly clear. Its like Matlab wearing a python costume.

Well, you know, they are pretty much the same programming language.

WEC-Sim can conflict with WAFO if they are both on the path at the same time, and since you might share some of the user base, I personally like having to manually add WAFO to my path here rather than a startup file, but I could see the benefits either way.

See my comments to Kelley on this.

I think for a version 0.1 release you guys look good!

Many thanks!

It would be great to see the example documentation fleshed out a little more...at least labeling the output figure (I think its power for each of the 8 query sea-states?).

Thanks, I've popped that comment into #87.

Thinking longer term, it would be great to see a more populated theory/workflow section to get a better idea of whats under the hood and what user I/Os are available.

I agree that some technical docs alongside would be great. Depends a bit on resources, I guess, and that is @ryancoe's burden.

I'm a little concerned about how extendable this meshing approach will be to non-axisymmetric geometries, as Nemoh leaves some to be desired on this front. If you intend to actively support this code I worry you'll have the same issues we do with WEC-Sim where half of our posted issues are actually Nemoh issues.

I guess it would be nice if we could be agnostic regarding the solver we called. This somewhat relates to an issue I am going to open regarding generalising mesh creation and it would be nice to be able deliver meshes to other tools that could give compatible results. If we could get it to work then it might well be useful beyond the scope of this toolbox.

H0R5E commented 4 years ago

I was able to install and run on windows, Matlab 2019a. However, my run of example had "Solver Stopped Prematurely" warnings for fmincon.

I was going to recommend against using fmincon because we've had issues with it before as well. I don't remember what the issue was, but I know it's a problematic function.

Not sure about this one. @ryancoe, @ssolson, @zmorrell-sand have you had any bother with this in production runs? It is used in the control optimisation too, so it would be bad if it's not reliable.

kmruehl commented 4 years ago

@H0R5E I'm going to try to reply inline as well, we will see how it goes

@kmruehl, I'm going to reply inline

Okay, I'm going to provide my feedback in a few comments. This is what I have thus far.

Index

  • Add a more detailed description about what the WecOptTool is and what it's used for. Why do people want to use this tool? What can it do for them? Why should they bother installing/using this toolbox? A user may think it looks like a lot of work to install, and they don't know why they would want to use it.

Yeah, I think we need this.

  • Contents/User Guide headers are redundant

Agreed, I have this structure to allow for multiple major sections (like User vs Technical) but it's not needed right now.

Installation

  • "Stable or development versions are available." Where is development, in a dev branch? On a fork? Will development be on master, so the "stable" version is the tagged release? How do I contribute do its development?

Contribution instructions are in the README, and we should add a note pointing people there for interested parties

  • I recommend using the .. Note syntax for notes
  • Specify version of the dependency, will the MATLAB Optimization Toolbox from 2015b work? I recommend specifying the version you've run as a minimum requirement

Yep, this is important.

  • The first step to remove an existing installation is confusing. I recommend making that a note. First step should be download the code via a clone or download.

OK, we'll take another look at this, thanks. Bad things happen if you install two versions.

  • For WEC-Sim we have a 'wecsimstartup.m' file that adds the path, see documentation here, you may consider having a startup file in the distribution with a relative path to add the path for the toolbox. You could also check for dependencies. Path issues are one the biggest issues we see, so the easier you can make it on users, the better.

Personally, I don't think startup files are any more reliable given the user may wish to change the startup directory depending on what they are working on at any one time. If there were a single unified location where these could be placed then that would be more useful, but the user still has to do some stuff. Our plan is take the next step in functionality with #58.

That's a fair point, and ultimately a decision up to the code deveopment team. The <Documents/MATLAB/startup.m> file is automatically executed when MATLAB is opened, so modifying it and adding the code's local /source path is the approach we took for WEC-Sim.

  • Should be addpath(genpath('C:\Users\kmruehl\Documents\GitHub\WecOptTool\wecopttool_v01_rc3\wecopttool')), otherwise subfolders are not added to directory.

See #84.

  • The savepath command gave me a weird warning. May consider having users create a 'startup.m' file in their MATLAB path that gets executed every time MATLAB is opened instead of saving the path

What did the warning say? As I said, not sure I see the relative value of a startup file.

It was a warning about having admin rights on the machine, I just hit accept. My concern with saving the path is that if you change the path, you'll have to remove the current path and and then add a new one (otherwise you'll have both versions of the code in your path). That's the reason we chose to add the path every time MATLAB is opened by modifying the <Documents/MATLAB/startup.m> file, instead of saving it. Once again, this is just a code development preference comment.

  • update documentation to specify correct NEMOH bin file directory, it's C:\Users\kmruehl\Documents\GitHub\Nemoh\Release, not Nemoh\bin

The documentation does give specific Windows vs Linux instructions, but we can try to make them more distinct by rolling them up.

Oh, is the Nemoh\bin the name of the LINUX directory? For Windows, it's Nemoh\Release

  • Until I added the genpath, I got this error, Undefined variable "WecOptLib" or class "WecOptLib.utils.getUserPath". when trying to install nemoh. I recommend having an output of the 'installNemoh' function that tells users the install was successful.
  • I like the dependencyCheck functionality, but I think it should be added to the end of the NEMOH install function and run automatically.

This is the subject of #69. I'll mark it high priority.

  • I like the runTests functionality, and the command window output it nice, but the documentation should also tell me what a successful 'runtests' looks like. Looks like I got a successful run Totals: 27 Passed, 0 Failed, 0 Incomplete.

Agreed. Thanks.

Copyright/License

  • Why was GNU open-source license selected? This will likely be limiting to developers who want to customize the toolbox for their application.

See the discussion in #60

  • Where is the Copyright assertion in the LICENSE file? <program> Copyright (C) <year> <name of author> This program comes with ABSOLUTELY NO WARRANTY; for details type 'show w'. This is free software, and you are welcome to redistribute it under certain conditions; type 'show c' for details. <one line to give the program's name and a brief idea of what it does.> Copyright (C) <year> <name of author>

I'm not sure you need to add this to the LICENSE file for GPL as every source file carries it and references to the text in licence file. The README should carry it though.

This should go through Sandia legal and have an NTESS assertion.

@ryancoe has this been done?

  • Copyright assertion also needs to be added to documentation: /wecopttool_v01_rc3/html_docs/user/license.html

I think the verbatim licence file should be replaced with the source code boilerplate.

  • Copyright assertion in source code should not be to "Sandia National Labs". This is what the assertion should look like: Copyright 2014 National Technology & Engineering Solutions of Sandia, LLC (NTESS). Under the terms of Contract DE-NA0003525 with NTESS, the U.S. Government retains certain rights in this software.

@ryancoe can you check this? We'll update it (in all the files - urg) if we have to.

Thanks again Kelley. I've summarised these in #85 and #86.

You're welcome, hope this feedback helps!

kmruehl commented 4 years ago

@H0R5E Attempting to respond in line here too...

Okay, and here's my second round of comments

Right, I've put all your notes regarding content in the example into #87. Many thanks. I'll just address some of the other operational things you posted.

Thank you, I'll review issue #87

Example

  • My MATLAB Workspace has a lot of variables in it. Is there an output object? If so, which one is it? Consider clearing all internal toolbox variables, and only keeping the inputs/ouputs in the workspace upon completed run

I think this is a drawback of having example.m as a script (but this is the most likely way it will be used) and I don't think it should be outputting anything that isn't in example.m. Are you seeing any variables being created that are not defined in example.m?

I added a screenshot of my MATLAB workspace to the comment, it may have been added after you saw the comments (I made a few minor revisions). I'm not sure if all of the variables are defined in the example.m, they probably are. Either way, I would recommend having an output object that clearly specifies the relevant I/O parameters and provide users with guidance on how to interact with that object. This is likely beyond the scope of an alpha release, but I think it would be very useful for users. Right now, after running the example, it was unclear what the result was and how to interact with it.

  • Why not use MHKiT instead of WAFO? This is also developed through WPTO funding and we can modify it as need be to support this program. It would be nice to see WPTO funded projects leverage other WPTO funded projects.

Good suggestion! Opened in #88.

  • The plot shows up as Figure 29, even though I have no other figures open.

Yeah, I've seen this too. Needs investigating. Opened in #89.

API Doc

  • We had a lots of issues implementing the API documentation for MHKiT-MATLAB when we had the source code on master and the documentation on gh-pages. I'm curious how you handled that.

So the plan is to store the sphinx source on master and push the build to gh-pages. You can follow #82 for the implementation until its merged. The next step is to automatically build the docs on a master branch merge. See #72 and #81.

I'll take a look at #82 . This is an issue we are currently trying to resolve with MHKiT-MATLAB. Right now we are compiling them in master and copying them over to gh-pages, which is a short-term solution, and has obvious drawbacks... including that we lose the index.

Tests

  • How are you running your unit tests? Do you have a runner on Jenkin? @zmorrell-sand just set up a Jenkins server with MATLAB to run WEC-Sim unit tests if you want to do the same thing for this tool

We are just running them manually at the moment and posting the run_tests.pdf file to our PRs for validation. It would be nice to get this running automagically, for sure.

Okay, let me/@zmorrell-sand how we can help. We have Jenkins set up for WEC-Sim, the server is on a virtual machine with MATLAB installed. It should be relatively simple to set up a Jenkins webhook for the WecOpTTool

General

  • I have a lot of questions about the overall theory/application of the code. It seems like it could be a very useful tool once a general geometry has been decided upon, and someone wants to refine its general dimension. Is that the intended use of this tool? This should be explained up front and be very clear. Users need to know why they would want to use this, and what it can accomplish.

Yeah, I think there is a balance of enticing without over-selling. This is just a beta, so we don't want to promise too much and not deliver otherwise people might not come back [ to DTOcean :'( ]. @ryancoe what's your thoughts here?

Understood. But if your goal is to target "WEC developers and researchers" you have to tell them why they care, and provide them the relevant theory/application. Similar comment as before, this is something that could/should be built upon for a future release. In my eyes, the goal of an alpha release is to get something functional out the door, examples/documentation are ongoing works in progress...

ryancoe commented 4 years ago

Thanks @kmruehl @zmorrell-sand @dforbush2! - I'm going to close out this issue as the key items have be split out into individual issues.

H0R5E commented 4 years ago

My concern with saving the path is that if you change the path, you'll have to remove the current path and and then add a new one (otherwise you'll have both versions of the code in your path).

@kmruehl, I see where you are coming from here. For us, this would only be an issue for those using the "development version" of the code, i.e. working from a cloned copy which they update. I also think its a legacy issue (although it seems to carry over to linux for some reason) because we should only need to add one folder (toolbox) and the package directories below should be automatically found (according to https://www.mathworks.com/help/matlab/matlab_oop/scoping-classes-with-packages.html#brfynt_-3).