-
Notifications
You must be signed in to change notification settings - Fork 11
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
Move theta input and change error check #397
Conversation
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.
The code changes are exactly right so as it stands this could be merged onto main. But...
- Could you write a short description of the PR (under the "conversation" tab)
- Do we want to think about renaming this? I've always wanted to clarify the difference between the following two schemes:
q^{n+1} = q^n + 0.5*dt*F(q^n) + 0.5*dt*F(q^{n+1})
and
q^{n+1} = q^n + dt*F(0.5*[q^n+q^{n+1}])
Since this one is the first, I had wondered if this should be called something like the TrapezoidalThetaMethod
? What do you think?
I've changed the name of ImplicitMidpoint |
I'm afraid the branch is still failing! I also wondered if we should open an issue to implement |
Yep I missed an |
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.
Looks good now, thanks!
theta is changed from an optional argument in
ThetaMethod
to required and limited to0 < theta < 1
.ImplicitMidpoint
is changed to be calledTrapeziumRule
. Note, in future we could add the actual Implicit Method in future.