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

undo times vector change #119

Closed
wants to merge 1 commit into from
Closed

Conversation

mforets
Copy link
Contributor

@mforets mforets commented May 28, 2021

No description provided.

@@ -119,7 +119,7 @@ end
# for its = 1:10
# # Remainder of Picard iteration
# Δ = picard_remainder!(f!, t, x, dx, xxI, dxxI, δI, δt, Δx, Δ0, params)
#
#
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this and the other empty character changes are automatic from my text editor, sorry for the noise..

@coveralls
Copy link

coveralls commented May 28, 2021

Pull Request Test Coverage Report for Build 884375509

  • 3 of 5 (60.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 63.361%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/validatedODEs.jl 3 5 60.0%
Totals Coverage Status
Change from base Build 883408675: 0.0%
Covered Lines: 773
Relevant Lines: 1220

💛 - Coveralls

@lbenet
Copy link
Member

lbenet commented May 28, 2021

I don't think this change is necessary, as I explained in this comment. The current (master) behavior is more consistent with respect to indexing, in such a way that the returned time at step n corresponds to the origin of the domain of the n-th entry of the solution, while before, it was the n-1 time entry.

@mforets
Copy link
Contributor Author

mforets commented May 28, 2021

but then, the length should be n+1? (or don't return the first reachable set at the time point t0). otherwise, we don't know by looking only at tTM when does the flowpipe end as i showed in this example

@lbenet
Copy link
Member

lbenet commented May 28, 2021

The length is actually maxsteps+1 (see here). The extra 1 comes from the initial condition, which i kept because it's sometimes handy to use directly the TaylorModel1 of that initial condition, as we do it in the tests (see here).

See #117 to see how everything becomes more consistent.

@mforets
Copy link
Contributor Author

mforets commented May 28, 2021

i still think that it is unexpected to just see two zeros in the example above instead of [0, 0, 0.1] (because validated_integ returns a view of only the maxsteps terms of tTM, not the full maxsteps+1) but ok 👍

@mforets mforets closed this May 28, 2021
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

3 participants