-
Notifications
You must be signed in to change notification settings - Fork 115
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
Adding Dynamic Selection RFC #2079
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly minor comments.
I think this is a nice distillation.
// (4) each submission can be waited on using the returned object | ||
ex::wait(done); | ||
} | ||
|
||
// (5) and/or all submissions can be waited on as a group | ||
ex::wait(p.get_submission_group()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to make more clear that in a normal case, one would use 4 or 5, rather than a combination of both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
to the Backend. The handle `h` is the mechanism for the Backend to report runtime | ||
information required by the Policy logic for making future selections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to mention here a situation where runtime information being reported to the policy is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
auto &r = resources_[i]; | ||
return selection_type{*this, r}; | ||
} else { | ||
throw std::logic_error(“selected called before initialization”); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems "selected" is intentional since it is repeated, and probably coming from the code.
I think we would want "selected before initialization" or "select called before initialization".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
|
||
May throw `std::bad_alloc` or `std::logic_error`. | ||
|
||
A call to `get_resources` may cause lazy initialization. Initialization may throw `std::bad_alloc`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably need to define "lazy initialization" for a backend (and its resources).
Co-authored-by: Dan Hoeflinger <109972525+danhoeflinger@users.noreply.github.com>
Co-authored-by: Dan Hoeflinger <109972525+danhoeflinger@users.noreply.github.com>
Co-authored-by: Dan Hoeflinger <109972525+danhoeflinger@users.noreply.github.com>
Co-authored-by: Dan Hoeflinger <109972525+danhoeflinger@users.noreply.github.com>
Co-authored-by: Dan Hoeflinger <109972525+danhoeflinger@users.noreply.github.com>
Co-authored-by: Dan Hoeflinger <109972525+danhoeflinger@users.noreply.github.com>
A description of the Dynamic Selection experimental feature. The intent is to capture the current design so that future RFCs can propose changes.