-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[air output] Update wording to "Logical Resource Usage". #33312
[air output] Update wording to "Logical Resource Usage". #33312
Conversation
Update wording to "Logical Resource Usage" and remove heap/object memory stuff. Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com>
Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com>
…0/ray into tune_output_resources
bunch of rllib test failures that I don't think relevant. |
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.
Code looks good, just two questions:
- Is "Logical Resource Usage" the correct term here? I don't know what it means. What is a logical resource? Maybe "Resources used" or "Assigned resources" or so?
- Can we make sure this is found in the documentation somewhere?
"Logical Resource" is in contrast to "Physical Resource". @scottsun94 in his user research, encountered people confused about this. Hence the emphasis on "Logical". "Usage" is also surfaced as preference from user research cc @scottsun94 I have no preference on any of the terms (apart from "Requested" is clearly misleading) - just wants to follow design suggestions here. Agree that we should make sure that our terminology is consistent. @scottsun94 do you know where ray doc uses those terms? |
Actually @krfricke I found it here! So seems a valid terminology to me! |
In Ray core, we already have a dedicated "Resource" doc page and talk about the difference between "logical resource" and "physical resource" The gap I found is that people using AIR don't really understand the distinction between logical resources and physical hardware utilization. For example, in my chat with relatively new ray users like @woshiyyya, they don't really know that "resource" refers to the logical resource concept purely for scheduling. In the long term, we probably need to introduce the "ray resource" or "logical resource" concept into AIR's doc because people shouldn't have to look at ray core doc to use AIR. I'm fine with going with "Resource usage" for now, being a bit ambiguous before we add the related doc into AIR. @richardliaw Any thoughts here on how we could better fill the gap? |
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.
Logical resource usage is fine with me if we consistently use this term and users understand it.
Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com>
Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com>
…#33312) * [air output] Update wording to "Logical Resource Usage". Update wording to "Logical Resource Usage" and remove heap/object memory stuff. Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * further ax a few extra stuff. * remove runner.debug_str Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> --------- Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: Jack He <jackhe2345@gmail.com>
…#33312) * [air output] Update wording to "Logical Resource Usage". Update wording to "Logical Resource Usage" and remove heap/object memory stuff. Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * further ax a few extra stuff. * remove runner.debug_str Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> --------- Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
…#33312) * [air output] Update wording to "Logical Resource Usage". Update wording to "Logical Resource Usage" and remove heap/object memory stuff. Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * further ax a few extra stuff. * remove runner.debug_str Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> --------- Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com>
…#33312) * [air output] Update wording to "Logical Resource Usage". Update wording to "Logical Resource Usage" and remove heap/object memory stuff. Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * further ax a few extra stuff. * remove runner.debug_str Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> --------- Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: elliottower <elliot@elliottower.com>
…#33312) * [air output] Update wording to "Logical Resource Usage". Update wording to "Logical Resource Usage" and remove heap/object memory stuff. Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * further ax a few extra stuff. * remove runner.debug_str Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> * Update python/ray/tune/utils/resource_updater.py Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> --------- Signed-off-by: xwjiang2010 <xwjiang2010@gmail.com> Signed-off-by: xwjiang2010 <87673679+xwjiang2010@users.noreply.github.com> Co-authored-by: Kai Fricke <krfricke@users.noreply.github.com> Signed-off-by: Jack He <jackhe2345@gmail.com>
Update wording to "Logical Resource Usage" and remove heap/object memory stuff.
Remove memory_debug_str on head node. Remove runner.debug_str (dangling code path and duplicated logic with what's used in
tune.run
- I don't think people will debug trial_runner by itself these days)Why are these changes needed?
Related issue number
#33152
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.