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 to SimControl::finalize function to correct SimpleCollisionMetrics output #504

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christomaszewski
Copy link
Contributor

Added a final call to run_networks() and then trigger the metric callbacks after the EntityPresentAtEnd messages are published in SimControl::finalize(). This fixes the bug where these messages never arrive to the SimpleCollisionMetrics plugin, which causes all teams to always show up as not having survived.

This fix also seems to correct the total flight time metric for the single agent during runs of the straight mission when it survives. The length of the straight mission is 100 seconds, so if the single agent team survives, the expected total flight time for that team is also 100. Prior to making this change I was getting total flight times of 53 (I guess this might vary but it seems pretty stable for me) when the single agent team survives. After the fix the total flight time shows 99.9.

…backs after the EntityPresentAtEnd messages are published in SimControl::finalize(). This fixes the bug where these messages never arrive to the SimpleCollisionMetrics plugin, which causes all teams to always show up as not having survived. This fix also seems to correct the total flight time metric for the single agent during runs of the straight mission when it survives - the mission length is 100 seconds, so if the agent survives until the end the expected total flight time is also 100 (with this fix it shows up as 99.9 vs 53 without it).
@christomaszewski
Copy link
Contributor Author

Any thoughts on this fix? I'd like to update the tutorials so that new users can follow along and match their output with the output shown in the tutorials when running with a deterministic seed. Are metrics plugins just generally not used or has someone fixed this issue on a private repo already? It seems like this might cause issues if you rely on all the messages sent in finalize actually getting through to the metrics plugin.

@christomaszewski
Copy link
Contributor Author

Can someone trigger CI on this again. There's no reason this should be failing on Ubuntu 20.04.

@mday299
Copy link

mday299 commented Apr 23, 2021

Did this PR fix it?
#511

@christomaszewski
Copy link
Contributor Author

Did this PR fix it?
#511

I can give it a shot but I don't see anything in the commits from that PR that specifically addresses the issue. I'll try to test it and report back.

@tm132
Copy link
Contributor

tm132 commented Jul 6, 2021

Can someone trigger CI on this again. There's no reason this should be failing on Ubuntu 20.04.

The CI on 20.04 failed due to docker rate limit, which isn't really related to the code changes. Admins (@mday299 or @frazierbaker etc), has the CI since this last failed been updated to work around this problem? If so, can one of the admins please kick off a rebuild to get this passing? This seems like a change that should be merged in.
https://travis-ci.com/github/gtri/scrimmage/jobs/469189352

library/ubuntu

toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit

The command "docker build -t $REPO:focal-${TRAVIS_BUILD_NUMBER} -f ./ci/dockerfiles/ubuntu-20.04 ." exited with 1.

0.00s$ export TAG=`if [ "$TRAVIS_BRANCH" == "master" ] && [ "$TRAVIS_PULL_REQUEST" == "false" ]; then echo "latest"; else echo $TRAVIS_BRANCH ; fi`

@christomaszewski
Copy link
Contributor Author

https://travis-ci.com/github/gtri/scrimmage/jobs/469189352

Can we pull this in :)

@mday299
Copy link

mday299 commented Oct 23, 2021

@christomaszewski I am no longer at GTRI and no longer actively maintaining scrimmage at this time.

@frazierbaker @shaun-d-anderson

Can you assist this gentleman?

@frazierbaker frazierbaker self-requested a review November 19, 2021 17:05
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.

3 participants