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

Analyzer: Add time-filtering functionality #128

Closed
wants to merge 2 commits into from

Conversation

EwoutH
Copy link
Contributor

@EwoutH EwoutH commented Sep 13, 2024

This PR introduces a new time-filtering functionality for analyzing link and vehicle data over a specified time window (t_seconds). This allows for more granular, time-sensitive analysis, which is particularly useful when evaluating the most recent simulation behavior. It basically allows inputting t_seconds into vehicles_to_pandas, link_to_pandas and area_to_pandas, which makes it collect the last t_seconds of simulation time.

It's a clean implementation of PR #123 and is part of issue #120.

A few things to note:

  • The t_seconds API is maybe still not ideal. Maybe you want to either:
    • Input t_start and t_end, and collect between that period.
    • Input a binwidth t_bin, and then slice up the data in equal parts of that width.
  • It only touches the Analyzer class, so in theory can now be run at the end of a simulation instead of during it. For that the API above might make more sense.

Things to review:

  • I think I'm triggering this at t=900 simulation time, but for some reason it takes t=930. Maybe the index is wrong by 1 or something like that
  • I had to add the or t_seconds is not None to the if s.flag_pandas_convert == 0 or t_seconds is not None: check in vehicles_to_pandas. Might be more elegant ways to solve it.

This branch can be installed with:

pip install -U -e git+https://github.com/EwoutH/uxsim@pandas_time_filter#egg=uxsim

@EwoutH
Copy link
Contributor Author

EwoutH commented Sep 14, 2024

Okay, this is clearly something that can better be done with an time_bin parameter after analysis.

Back to the drawing board.

@toruseo
Copy link
Owner

toruseo commented Sep 16, 2024

If we choose time_bin approach, this PR can be closed? Just comfirming

@EwoutH EwoutH marked this pull request as draft September 16, 2024 03:23
@EwoutH
Copy link
Contributor Author

EwoutH commented Sep 16, 2024

Yes, succeeded by #130.

@EwoutH EwoutH closed this Sep 16, 2024
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.

2 participants