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

New experimental simulation runner #4161

Merged
merged 3 commits into from
Apr 23, 2019

Conversation

alessio
Copy link
Contributor

@alessio alessio commented Apr 19, 2019

runsim is a tool desinged to tackle issues the team at times struggle with while working with multisim.sh:

  • rate limiting: by default runsim spawn min(10,len(seeds)) processes. Max number of processes can be passed via -j.
  • Spawns a process each simulation and correctly repeals it upon program termination, regardless of what caused the program to terminate (SIGINT, SIGTERM, seeds exhaustion).

It must be noted that it is not fully ABI-compatible with multisim.sh:

  • Different output format.
  • Genesis file path must be passed via the -g flag.
  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote tests
  • Updated relevant documentation (docs/)
  • Added a relevant changelog entry: sdkch add [section] [stanza] [message]
  • rereviewed Files changed in the github PR explorer

For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Apr 19, 2019

Codecov Report

Merging #4161 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #4161   +/-   ##
========================================
  Coverage    60.19%   60.19%           
========================================
  Files          212      212           
  Lines        15157    15157           
========================================
  Hits          9124     9124           
  Misses        5409     5409           
  Partials       624      624

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

TestedACK, but a minor nit:

Most of the output has [Wn] as a prefix. It would be nice if the Seed n OK|FAILED also had this prefix for consistency. Would that be possible?

cmd/gaia/contrib/runsim/main.go Outdated Show resolved Hide resolved
cmd/gaia/contrib/runsim/main.go Outdated Show resolved Hide resolved
Copy link
Contributor

@sabau sabau left a comment

Choose a reason for hiding this comment

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

Tested locally, all good. I probably need more context about the timeline for splitting repos

@alexanderbez alexanderbez merged commit 4bf9d2b into develop Apr 23, 2019
@alexanderbez alexanderbez deleted the alessio/simulation-parallel-runner branch April 23, 2019 13:02
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