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

Do not reclassify ferry connections when hierarchy=false #4487

Merged
merged 17 commits into from
Jan 11, 2024
Merged

Conversation

dnesbitt61
Copy link
Member

@dnesbitt61 dnesbitt61 commented Jan 3, 2024

Do not reclassify ferry connections if hierarchies are not being used. Add a log statement to indicate no reclassification is occurring in the case where mjolnir config has hierarchy set to false.

Update graphbuilder to call link reclassification if infer_turn_channels is true. However, if reclassify_links is false then the actual reclassification (changing an edges classification) is bypassed but turn channel generation is performed (if config indicates infer_turn_channels=true).

fixes #4486

Log statements from a Maryland build where infer_turn_channels=true, reclassify_links=false, and hierarchy=false:

2024/01/10 13:16:49.685371 [INFO] Finished with 0 link edges reclassified.  Turn channel count = 4935
2024/01/10 13:16:49.709464 [WARN] Not reclassifying ferry connections since no hierarches are being created

Issue

What issue is this PR targeting? If there is no issue that addresses the problem, please open a corresponding issue and link it here.

Tasklist

  • Add tests
  • Add #fixes with the issue number that this PR addresses
  • Update the docs with any new request parameters or changes to behavior described
  • Update the changelog
  • If you made changes to the lua files, update the taginfo too.

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

being used. Add a log statement to indicate no reclassification is
occurring in the case where mjolnir config has hierarchy set to false.
nilsnolde
nilsnolde previously approved these changes Jan 3, 2024
Copy link
Member

@nilsnolde nilsnolde left a comment

Choose a reason for hiding this comment

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

LGTM

this method in case turn channels are being inferred, this allows a
config to have reclassify_links = false and infer_turn_channels = true.
In this case, turn channels are marked but no edges are reclassified.
@dnesbitt61
Copy link
Member Author

Updated so that ReclassifyLinks is called if either reclassify_links and infer_turn_channels is true in the config. Added a reclassify_links bool to ReclassifyLinks (and pass this through to ReclassifyLinkGraph). If this is false, no edges are reclassified (but can be used for infer_turn_channels).

@dnesbitt61 dnesbitt61 changed the title Do not reclassify ferry connections or links when hierarchy=false Do not reclassify ferry connections when hierarchy=false Jan 9, 2024
kevinkreiser
kevinkreiser previously approved these changes Jan 10, 2024
Copy link
Member

@kevinkreiser kevinkreiser left a comment

Choose a reason for hiding this comment

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

should have some tests for this but it is very straight forward

@dnesbitt61
Copy link
Member Author

I copied the TEST(RampsTCs, test_tc_infer) test and changed so that reclassify_links is false and infer_turn_channels is true. The test checks the ramps classification (makes sure they stay as motorway). Edge GK seems to still be marked as internal and a turn channel (as before). However, the narrative checks fail now. I am not sure why!

kevinkreiser
kevinkreiser previously approved these changes Jan 10, 2024
@dnesbitt61 dnesbitt61 merged commit 5540cd7 into master Jan 11, 2024
9 checks passed
@dnesbitt61 dnesbitt61 deleted the no_reclass branch January 11, 2024 17:39
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.

When hierarchies are not built, we should not reclassify links or ferry connections
3 participants