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.

Sign up for GitHub

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

Jump to bottom

Hl/#1654 H3 Skimmer + Simple relocation algorithm for veh sharing #2264

Merged
merged 41 commits into from
Jan 7, 2020
Merged

Hl/#1654 H3 Skimmer + Simple relocation algorithm for veh sharing #2264

merged 41 commits into from
Jan 7, 2020

Conversation

haitamlaarabi
Copy link
Collaborator

@haitamlaarabi haitamlaarabi commented Nov 5, 2019 •
edited by JustinPihony
Loading

I added new versions of the skimmers that are basically EventHandlers. The Companions of these Skimmers serve as Read Only Access to the past skims that have been collected at the end of the previous iterations.

Closes #1654

This change is Reviewable

colinsheppard
colinsheppard suggested changes Nov 5, 2019
View reviewed changes
Copy link
Contributor

@colinsheppard colinsheppard left a comment

Choose a reason for hiding this comment

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

Do we have any test cases that just confirm the different skimmers are summarizing results correctly? E.g. we could do something like run beamville and compare some metric in the skims file to one calculated by catching events.

src/main/resources/beam-template.conf Outdated Show resolved Hide resolved
src/main/resources/beam-template.conf Outdated Show resolved Hide resolved
JustinPihony
JustinPihony requested changes Nov 5, 2019
View reviewed changes
Copy link
Collaborator

@JustinPihony JustinPihony left a comment

Choose a reason for hiding this comment

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

A few places of minor change needed - most are style or minor otherwise.

src/main/scala/beam/agentsim/infrastructure/taz/H3TAZ.scala Outdated Show resolved Hide resolved
src/main/scala/beam/agentsim/infrastructure/taz/H3TAZ.scala Outdated Show resolved Hide resolved
src/main/scala/beam/agentsim/infrastructure/taz/H3TAZ.scala Outdated Show resolved Hide resolved
src/main/scala/beam/agentsim/infrastructure/taz/H3TAZ.scala Outdated Show resolved Hide resolved
src/main/scala/beam/router/skim/AbstractSkimmer.scala Show resolved Hide resolved
src/main/scala/beam/router/skim/ODSkimmer.scala Outdated Show resolved Hide resolved
src/main/scala/beam/router/skim/ODSkimmer.scala Outdated Show resolved Hide resolved
src/main/scala/beam/router/skim/ODSkimmer.scala Outdated Show resolved Hide resolved
src/main/scala/beam/router/skim/ODSkimmer.scala Outdated Show resolved Hide resolved
src/main/scala/beam/router/skim/ODSkimmer.scala Outdated Show resolved Hide resolved
@haitamlaarabi
Copy link
Collaborator Author

test!

@haitamlaarabi
Copy link
Collaborator Author

test!

@JustinPihony
Copy link
Collaborator

test!

JustinPihony
JustinPihony approved these changes Nov 25, 2019
View reviewed changes
Copy link
Collaborator

@JustinPihony JustinPihony left a comment

Choose a reason for hiding this comment

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

There are still a number of asInstanceOf which could blow up...but are probably fine...

src/test/scala/beam/agentsim/infrastructure/H3TAZSpec.scala Outdated Show resolved Hide resolved
src/test/scala/beam/router/skim/ODSkimmerSpec.scala Outdated Show resolved Hide resolved
src/main/scala/beam/router/skim/AbstractSkimmer.scala Outdated Show resolved Hide resolved
src/main/scala/beam/router/skim/CountSkimmer.scala Outdated Show resolved Hide resolved
@haitamlaarabi
Copy link
Collaborator Author

test!

@haitamlaarabi
Copy link
Collaborator Author

test!

@haitamlaarabi
Copy link
Collaborator Author

test!

@haitamlaarabi
Copy link
Collaborator Author

test!

@haitamlaarabi
Copy link
Collaborator Author

Test!

@JustinPihony
Copy link
Collaborator

test!

1 similar comment
@JustinPihony
Copy link
Collaborator

test!

@haitamlaarabi
Copy link
Collaborator Author

Test!

@colinsheppard colinsheppard merged commit d40faa4 into develop Jan 7, 2020
@colinsheppard colinsheppard deleted the HL/#1654-beam-h3-bis branch January 7, 2020 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JustinPihony JustinPihony JustinPihony approved these changes

@colinsheppard colinsheppard colinsheppard approved these changes

@michaz michaz Awaiting requested review from michaz

@wrashid wrashid Awaiting requested review from wrashid

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

switch to H3 from TAZ
4 participants
@haitamlaarabi @JustinPihony @colinsheppard @REASY