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

implement solveODE #2958

Merged
merged 20 commits into from
Jul 12, 2023
Merged

implement solveODE #2958

merged 20 commits into from
Jul 12, 2023

Conversation

dvd101x
Copy link
Sponsor Collaborator

@dvd101x dvd101x commented May 29, 2023

Hi, this includes a function to do numeric integration of ordinary differential equations in a similar fashion to scipy.integrate.solve_ivp.

It includes Runge-Kutta methods with variable step size 'RK45' and 'RK23'.

Some ideas for the future are:

  • Include an array like for tolerance for each element in the state (currently it calculates only with the maximum value of the error)
  • Include tEval option so that it guaranties that every one of the times in that array are returned and no other one (internal steps might vary).
  • args: include extra arguments to the function so if a function already exists of the form myF(t, y, a, b, c) we could include in options {args: [a, b, c]} instead of making a new function of the form f(t, y)
  • Once there is a root finding algorithm in numeric, implement other methods like:
    • BDF
    • LSODA

Copy link
Owner

@josdejong josdejong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot David, this looks really neat. The code looks very clean and easy to follow. I made a couple of inline comments, can you have a look at those? (nothing big)

src/function/numeric/solveODE.js Show resolved Hide resolved
test/unit-tests/function/numeric/solveODE.test.js Outdated Show resolved Hide resolved
test/unit-tests/function/numeric/solveODE.test.js Outdated Show resolved Hide resolved
src/function/numeric/solveODE.js Outdated Show resolved Hide resolved
src/function/numeric/solveODE.js Show resolved Hide resolved
@josdejong
Copy link
Owner

O, and I see one of the CI tests fails because there are no embedded docs. Can you add a file solveODE.js there and list it in src/expression/embeddedDocs.js? Thanks!

@dvd101x
Copy link
Sponsor Collaborator Author

dvd101x commented Jun 13, 2023

Hi Jos, thank you for your through review.

Everything is very clear, I will work on that, I just included a few comments regarding error messages.

@dvd101x
Copy link
Sponsor Collaborator Author

dvd101x commented Jul 2, 2023

Thank you very much for the review. Here is an overview of the changes

  • It has embedded docs
  • Included the returned object in the jsdocs
  • It works with matrices and returns matrices
  • Throws some more specific errors regarding
    • max iter reached
    • tspan has no numeric or unit values
    • method name
  • Simplifies most tests using functions previously implemented
  • Included a more challenging function to test f(t, y) = y - t instead of f(t, y) = y

Here is the bad part:

  • It doesn't work with big numbers yet

@dvd101x
Copy link
Sponsor Collaborator Author

dvd101x commented Jul 2, 2023

Now it works with big numbers

@dvd101x
Copy link
Sponsor Collaborator Author

dvd101x commented Jul 6, 2023

These are the recent changes:

  • The logic was simplified a bit by forcing:
    • maxStep and firstStep to be positive
    • minStep to be positive or zero
  • Included tests for time related variables like tspan[0], tspan[1], minStep, maxStep and firstStep
  • I encountered an error where I thought isNumeric included only number and bigNumber. So I used something else
  • Fixed a typo with deltaB

Some drawbacks is that now instead of just solving some cases it throws an error, bit it might be ok.

@dvd101x dvd101x requested a review from josdejong July 8, 2023 15:26
@josdejong
Copy link
Owner

Thanks for the updates David. All looks good to go 👌

@josdejong josdejong merged commit 1aed900 into josdejong:develop Jul 12, 2023
8 checks passed
@dvd101x
Copy link
Sponsor Collaborator Author

dvd101x commented Jul 12, 2023

That's great news! Thanks!

@josdejong
Copy link
Owner

Published now in v11.9.0.

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

Successfully merging this pull request may close these issues.

None yet

2 participants