-
-
Notifications
You must be signed in to change notification settings - Fork 60
Docs Reorganization #280
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
Docs Reorganization #280
Conversation
|
||
## Basic usage | ||
|
||
```julia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have choosen a case I didn't include right now. 😅
Right now, SINDy-like methods have been dispatched on continuous, but this should be an easy fix in here, where DX
has to be changed to get_target(prob)
.
I will commit during the day and add more test cases for Discrete and Direct Problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha cool, glad I wasn't missing something obvious. Maybe there should be a fallback method for combinations that aren't implemented that throws an error? If you point me in the right direction, I can take a stab at this.
|
||
## Choosing a Basis | ||
|
||
A basis is optional, depending on the solver and solution method you are using. For instance, for DMD, a basis is not required, but for SINDy using STLQS(), it is required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, but this should also be optional ( note for me ).
@@ -0,0 +1,85 @@ | |||
# Unifying SINDy and DMD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree to put a spotlight on SINDy + DMD, but maybe this should be moved into a Math
Section and here we highlight Common Solver Interface
? What's your opinion on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think people are going to be searching for in the docs? I'm not an expert in this field, but my impression is that most people think in terms of SINDy and DMD, and would search based on those terms. If that's the case, I think it's good to have those terms in the headers and side bar. However, if there's a more common search term, we should highlight that instead. I think we can highlight the common solver interface elsewhere, maybe in the getting started page. I don't think it's as important, because it's more of an engineering detail than a use case.
This is just my two cents though: you're the expert, and I'm happy to do whatever you feel is appropriate.
Hey Greg! Very nice work so far! Thank you! I added some minor discussion points / proposals for changes and reminder for myself 😅 . For your list above:
I am fully back and just adjusting to be fully back right now, so feel free to DM on Slack or keep this conversation going. |
Ah, and I compiled an initial list of references here. Once you feel that this PR is mature enough, I can jump in and add it and DocumenterCitations.jl . |
Ok, I think this PR is in a decent place. I'd still like to check it using your LiveServer.jl workflow, so I can make sure the png's and optional module elements are working properly. But, I think most things are in and working, and stuff is reasonably proofread at this point. |
This PR reorganizes the docs as suggested in #247. I've taken a lot of liberties in reorganizing, with the rough outline as follows:
To me, this outline made the most sense because the general workflow is:
However, let me know if you'd like something different.
This PR still has some outstanding issues and questions:
monomial_basis
to return a basis, not a vector of equations. I'll call that out in the docs, but it might be clearer to name them something likemonomial_basis_eqs
or something like that. [Addressed below]