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

robot wave scenario #1556

Merged
merged 4 commits into from
Oct 1, 2023
Merged

robot wave scenario #1556

merged 4 commits into from
Oct 1, 2023

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Sep 30, 2023

image

Demo

scripts/play.sh -i data/scenarios/Challenges/wave.yaml --autoplay

@kostmo kostmo added G-Scenarios An issue having to do with scenario design, the way scenarios are described and loaded, etc. CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. labels Sep 30, 2023
@kostmo kostmo changed the title wave robot wave Sep 30, 2023
@kostmo kostmo changed the title robot wave robot wave scenario Sep 30, 2023
@kostmo kostmo marked this pull request as ready for review September 30, 2023 05:27
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

Cool stuff @kostmo, I could see this turn into a full Arcade run with multiple levels. 🚀

dsl: |
overlay
[ {dirt, water}
, if x % 7 + y % 5 == 0 then {dirt, wavy water} else {blank}
Copy link
Member

Choose a reason for hiding this comment

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

Nice, I like the repeating pattern, it looks like old school games. 🎮

def doN = \n. \f. if (n > 0) {f; doN (n - 1) f} {}; end;

def crossPath =
doN 6 move;
Copy link
Member

@xsebek xsebek Sep 30, 2023

Choose a reason for hiding this comment

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

I suspect this is needlessly slow because we do not inline it.

Suggested change
doN 6 move;
// doN 6 move;
move; move; move; move; move; move;

I also tried replacing wait with turn forward. But do not do this, I measured it and wait is better!

Subjectively, the simulation seemed to keep playing better without lag on my computer once inlined.

EDIT: I will try to rebuild with profiling enabled and see if the difference is measurable.

Copy link
Member

Choose a reason for hiding this comment

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

Just following a blog post about profiling with cabal, I can see some difference in the graphs:

Non-Inlined Inlined
swarm_noninlined swarm_inlined

Copy link
Member

Choose a reason for hiding this comment

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

@xsebek Great work looking at performance, memory profiling, etc.! We haven't paid much attention to it so far but I'm sure there is tons of low-hanging fruit to improve performance.

I'm not sure inlining things in scenario programs is the right way to go, though. Ideally we should be able to encourage nice, modular code and work hard to make that nice, abstracted, modular code run fast.

Copy link
Member

@xsebek xsebek Sep 30, 2023

Choose a reason for hiding this comment

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

@byorgey it’s definitely not ideal to do it manually, thats why I made an issue to do it automatically. 🙂

@kostmo I would suggest inlining and adding a TODO linking the inline issue, but up to you.

x............................................................................................................................................................................................................................................................x
xxx........................................................................................................................................................................................................................................................xxx
xxxxxxwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwxxxxxx

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this could be a bit shorter? Though it does work well as a performance test. 😅

data/scenarios/Challenges/wave.yaml Show resolved Hide resolved
data/scenarios/Challenges/wave.yaml Outdated Show resolved Hide resolved
@kostmo kostmo requested a review from xsebek October 1, 2023 04:57
@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Oct 1, 2023
@mergify mergify bot merged commit c1d0fdd into main Oct 1, 2023
11 checks passed
@mergify mergify bot deleted the scenarios/wave branch October 1, 2023 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. G-Scenarios An issue having to do with scenario design, the way scenarios are described and loaded, etc. merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants