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

Ordering of dimensions is not consistent across parts of the package #24

Open
shengjiex98 opened this issue Mar 22, 2023 · 2 comments · May be fixed by #25
Open

Ordering of dimensions is not consistent across parts of the package #24

shengjiex98 opened this issue Mar 22, 2023 · 2 comments · May be fixed by #25

Comments

@shengjiex98
Copy link
Collaborator

In deviation(), the three dimensions for reachable_corners and nominal_trajectory are state variables, points, time; while in bounded_runs_iter(), the three dimensions for the return value are time, augmented state, min/max; finally, the return value of evol() has dimensions time, state variables.

Making these have the same ordering of dimensions will reduce the chance of making mistakes while passing arguments across functions, such as passing in a nominal trajectory to deviation() from evol(). Of course, we should wait until after the deadlines to make these changes.

@Ratfink
Copy link
Owner

Ratfink commented Mar 22, 2023

See also #8, which could be done at the same time.

@Ratfink
Copy link
Owner

Ratfink commented Mar 31, 2023

To match lsim from ControlSystems.jl, we should standardize on time being last. That has the advantage of making a single time instant be a consecutive block of memory, generally improving cache performance.

In the current work I'm doing on #8 (see the intervals branch), bounded_runs_iter now just returns a vector of IntervalBoxes. I think the order of the main arrays in the deviation function shouldn't change, and in fact I'd like to make a method of corners_from_bounds to make a vector of IntervalBoxes into such an array automatically. evol needs to change the order of its result. I'll also make it accept an IntervalBox as initial state, which would be handy for computing nominal trajectories.

@Ratfink Ratfink linked a pull request Apr 3, 2023 that will close this issue
7 tasks
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 a pull request may close this issue.

2 participants