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

DOCUMENTATION: Improve explanation of nsubsteps usage #314

Open
michaelcdevin opened this issue Jan 9, 2024 · 1 comment
Open

DOCUMENTATION: Improve explanation of nsubsteps usage #314

michaelcdevin opened this issue Jan 9, 2024 · 1 comment
Labels
documentation Improvements or additions to documentation

Comments

@michaelcdevin
Copy link
Collaborator

michaelcdevin commented Jan 9, 2024

Derived from #307.

To my understanding, WecOptTool currently uses substeps for two purposes:

  1. To enforce constraints at more frequencies than needed for the dynamics equation without excessive computational expense (see Feature request: apply nsubsteps to dynamics equality constraint #307 (comment))
  2. To discretize results for smoother plotting for post-processing

The documentation is rather indirect about these two uses: Tutorial 1 broadly points to the Theory documentation for an explanation of using substeps in constraints without specifying where, and then is a bit handwavy for its use for post-processing. The Theory documentation does include some explanation in the Constraints section):

An important practical factor when using this functionality is to make sure that the constraint is evaluated at a sufficient number of collocation points. It may be required to enforce constraints at more points than the dynamics (as defined by the frequency array).
In WecOptTool's example PTO module, this is controlled by the nsubsteps argument (see, e.g. wecopttool.pto.PTO.force_on_wec).

This could probably be improved, as "it may be required" could be changed to explain why you would want to use substeps specifically for the constraints instead of changing the frequency vector.

Overall, I think the use of nsubsteps could be clarified in a number of ways:

  • The reference in Tutorial 1 to the Theory documentation should be made into a hyperlink going directly to the Constraints section (I believe this used to be the case but was changed due to CI issues with Sphinx linkcheck, but this is not a concern since this was removed in Remove linkcheck #287)
  • The explanation in the Theory documentation could be made more specific, i.e. explain why "it may be required" to use substeps for the constraints
  • The difference in using nsubsteps for the constraints and for post-processing could be clarified, either by providing language for this in Tutorial 1, or perhaps by changing the variable name to something else for one of these, since it is two rather distinct uses for the same variable name
@michaelcdevin michaelcdevin added the documentation Improvements or additions to documentation label Jan 9, 2024
@ryancoe
Copy link
Collaborator

ryancoe commented Jan 10, 2024

Per discussion yesterday with @michaelcdevin @cmichelenstrofer @dtgaebe @gbacelli, will also be good to make clear the distinction between nsubsteps and nfreq (inequality constraints vs. equality constraints) -- related to #307.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants