argobots icon indicating copy to clipboard operation
argobots copied to clipboard

sched: add a registration-based scheduler to argobots

Open epmikida opened this issue 3 years ago • 2 comments

Pull Request Description

This adds a basic registration-based scheduler option to Argobots, which allows users to associate a scheduling policy with each pool that is passed to the scheduler which determines how the scheduler will select which pool to pop from at each iteration of the scheduler loop.

Checklist

  • [ ] Reference appropriate issues (with "Fixes" or "See" as appropriate)
  • [ ] Commits are self-contained and do not do two things at once
  • [ ] Commit message is of the form: module: short description and follows good practice
  • [ ] Passes whitespace checkers

epmikida avatar Jan 25 '22 21:01 epmikida

Thank you for your contribution, @epmikida!

Before diving into the detailed code review, can I ask you the following?

  1. Please add at least one small test in test/basic/ (in a different commit).
  2. Please fix all the whitespace/compilation problems. The CI tests will be run automatically whenever you push changes
  3. Please rebase the branch to the latest main branch (GitHub says This branch is out-of-date with the base branch)
  4. Please explain a typical use case of the new scheduler. It'd be great if you could give a small code snippet.
  5. Similar to 4: please explain merits of this new scheduler over the existing user-defined scheduler from a viewpoint of performance, abstraction, and/or functionality.
    • Particularly, do we need a new function for this new scheduler type? Isn't it another "predefined" scheduler (see ABT_sched_predef)?

(Please let me note that I am not opposing to the idea, and I believe I know the idea and like it!)

shintaro-iwasaki avatar Jan 26 '22 03:01 shintaro-iwasaki

Thank you for your contribution, @epmikida!

Before diving into the detailed code review, can I ask you the following?

  1. Please add at least one small test in test/basic/ (in a different commit).

  2. Please fix all the whitespace/compilation problems. The CI tests will be run automatically whenever you push changes

  3. Please rebase the branch to the latest main branch (GitHub says This branch is out-of-date with the base branch)

  4. Please explain a typical use case of the new scheduler. It'd be great if you could give a small code snippet.

  5. Similar to 4: please explain merits of this new scheduler over the existing user-defined scheduler from a viewpoint of performance, abstraction, and/or functionality.

    • Particularly, do we need a new function for this new scheduler type? Isn't it another "predefined" scheduler (see ABT_sched_predef)?

(Please let me note that I am not opposing to the idea, and I believe I know the idea and like it!)

Yes, I'll get working on these changes right away.

epmikida avatar Jan 31 '22 16:01 epmikida