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

Fix outcome_map disambiguation for Concurrence #80

Merged

Conversation

luisrayas3
Copy link

According to the documentation:

"""
If the criteria for one outcome is the subset of another outcome,
the container will choose the outcome which has more child outcome
criteria satisfied. If both container outcomes have the same
number of satisfied criteria, the behavior is undefined.
"""

However, this did not seem to be implemented as such, instead it appears
the behavior was undefined regardless of the number of satisfied
criteria. This fixes the outcome_map handling such that a new outcome
from the map is only accepted if it satisfies strictly more child labels
than any previously consider outcome, thus satisfying the documented
behavior.

According to the documentation:

"""
If the criteria for one outcome is the subset of another outcome,
the container will choose the outcome which has more child outcome
criteria satisfied. If both container outcomes have the same
number of satisfied criteria, the behavior is undefined.
"""

However, this did not seem to be implemented as such, instead it appears
the behavior was undefined regardless of the number of satisfied
criteria. This fixes the outcome_map handling such that a new outcome
from the map is only accepted if it satisfies strictly more child labels
than any previously consider outcome, thus satisfying the documented
behavior.
@luisrayas3 luisrayas3 force-pushed the luis/disambiguate-concurrence-outcome-map branch from 4a3a1c6 to 115ea23 Compare March 31, 2021 21:18
@130s 130s changed the title Implement outcome_map disambiguation for Concurrence Fix outcome_map disambiguation for Concurrence Jun 13, 2023
@130s
Copy link
Member

130s commented Jun 13, 2023

Thanks for contrubition.
I have not tested nor thoroughly reviewed but brief look the goal you describe seems achieved.

Merging and will depend on the community to further test.

@130s 130s merged commit 0a62d3e into ros:noetic-devel Jun 13, 2023
1 check passed
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

2 participants