[Core] Non Unit Instance fractional value fix #39293

jonathan-anyscale · 2023-09-06T01:23:35Z

Why are these changes needed?

There's a bug where Ray should've allow non unit instance resource to have fractional value greater than 1 (unit instance resource currently: GPU, TPU, Neuron Core). This PR is to fix that

Related issue number

Closes #37241

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

jjyao

LGTM!

jjyao · 2023-09-06T07:20:44Z

doc/source/ray-core/scheduling/resources.rst

@@ -194,3 +194,6 @@ The precision of the fractional resource requirement is 0.0001 so you should avo

  Besides resource requirements, you can also specify an environment for a task or actor to run in,
  which can include Python packages, local files, environment variables, and more---see :ref:`Runtime Environments <runtime-environments>` for details.
+
+.. note::
+  Fractional input must be between 0 to 1. For any value greater than 1, `num_cpus` and `num_gpus` requires a whole number.


Suggested change

Fractional input must be between 0 to 1. For any value greater than 1, `num_cpus` and `num_gpus` requires a whole number.

Fractional resource requirement must be smaller than 1. For any resource requirement greater than 1, it needs to be a whole number (e.g. ``num_cpus=1.5`` is invalid).

Can you move it to be above tip?

Signed-off-by: Jonathan Nitisastro <jonathannitisastro@jonathancn-QC69NQYVVG.local.meter>

…esource_doc

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

…esource_doc

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

krfricke

Stamp for docs

python/ray/_raylet.pyx

rkooo567 · 2023-09-12T07:32:41Z

doc/source/ray-core/scheduling/resources.rst

@@ -194,3 +194,6 @@ The precision of the fractional resource requirement is 0.0001 so you should avo

  Besides resource requirements, you can also specify an environment for a task or actor to run in,
  which can include Python packages, local files, environment variables, and more---see :ref:`Runtime Environments <runtime-environments>` for details.
+
+.. note::
+  For any unit resource (GPU, TPU, Neuron Core) requirement greater than 1, it needs to be a whole number (e.g. ``num_gpus=1.5`` is invalid).


can you add more details about what 's the definition of unit resource?

it is the internal terminology

rkooo567 · 2023-09-12T07:37:07Z

python/ray/tests/test_advanced_2.py


-    @ray.remote(num_cpus=1.5)
-    def test():
+    test_frac_cpu.remote()


Suggested change

test_frac_cpu.remote()

assert ray.get(test_frac_cpu.remote())

and make test_frac_cpu return True (it is better having explicit assert)

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

rkooo567 · 2023-09-15T01:04:11Z

lint failure

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

angelinalg

I hope the rewrite still makes sense. Let me know if you have questions.

doc/source/ray-core/scheduling/resources.rst

Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com> Signed-off-by: jonathan-anyscale <144177685+jonathan-anyscale@users.noreply.github.com>

rkooo567 · 2023-09-21T00:44:45Z

merge conflict

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

jjyao · 2023-09-24T14:50:51Z

doc/source/ray-core/scheduling/resources.rst

-  which can include Python packages, local files, environment variables, and more---see :ref:`Runtime Environments <runtime-environments>` for details.
+  which can include Python packages, local files, environment variables, and more. See :ref:`Runtime Environments <runtime-environments>` for details.
+
+.. note::


Can you move it to be above tip?

jjyao · 2023-09-24T14:59:41Z

doc/source/ray-core/scheduling/resources.rst

+  which can include Python packages, local files, environment variables, and more. See :ref:`Runtime Environments <runtime-environments>` for details.
+
+.. note::
+  Unit resource requirements that are greater than 1, 


People usually don't know what unit resources are. We should just list them out like

GPU, TPU, neuron_cores resource requirements that are greater than 1, need to be whole numbers. For example, ``num_gpus=1.5`` is invalid.

the GPU, TPU and neuron core are written on the next line

jjyao · 2023-09-24T15:08:28Z

python/ray/_raylet.pyx

+            unit_resources = f"{RayConfig.instance().predefined_unit_instance_resources()},\
+            {RayConfig.instance().custom_unit_instance_resources()}"


lets split by (,) and convert them to a set and then check. Otherwise, we may mismatch: e.g. we have a unit resource called Foo_Bar and you will think Foo is also unit resource.

jjyao · 2023-09-24T15:10:22Z

python/ray/_raylet.pyx

+                    "Unit instance resource (GPU, TPU, Neuron Core) quantities >1 must",
+                    f" be whole numbers. {key} resource with value {value} is invalid.")


Suggested change

"Unit instance resource (GPU, TPU, Neuron Core) quantities >1 must",

f" be whole numbers. {key} resource with value {value} is invalid.")

f"{key} resource quantities >1 must",

f" be whole numbers. The specified quantity {value} is invalid.")

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

doc/source/ray-core/scheduling/resources.rst

Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: jonathan-anyscale <144177685+jonathan-anyscale@users.noreply.github.com>

There's a bug where Ray should've allow non unit instance resource to have fractional value greater than 1 (unit instance resource currently: GPU, TPU, Neuron Core). This PR is to fix that Signed-off-by: Jonathan Nitisastro <jonathannitisastro@jonathancn-QC69NQYVVG.local.meter> Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com> Signed-off-by: jonathan-anyscale <144177685+jonathan-anyscale@users.noreply.github.com> Co-authored-by: Jonathan Nitisastro <jonathannitisastro@jonathancn-QC69NQYVVG.local.meter> Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com> Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: Victor <vctr.y.m@example.com>

jonathan-anyscale requested a review from a team as a code owner September 6, 2023 01:23

jonathan-anyscale force-pushed the fractional_resource_doc branch from a7c6b77 to 9fb8390 Compare September 6, 2023 01:35

jonathan-anyscale changed the title ~~Add small note on documentation for fragmented resource~~ [docs] Add small note on documentation for fragmented resource Sep 6, 2023

jjyao approved these changes Sep 6, 2023

View reviewed changes

jonathan-anyscale force-pushed the fractional_resource_doc branch from 9fb8390 to d3c2d9c Compare September 6, 2023 15:04

jjyao assigned angelinalg and jjyao Sep 6, 2023

Add small note on documentation for fragmented resource

b73674b

Signed-off-by: Jonathan Nitisastro <jonathannitisastro@jonathancn-QC69NQYVVG.local.meter>

jonathan-anyscale force-pushed the fractional_resource_doc branch from 7232c66 to b73674b Compare September 6, 2023 16:21

Jonathan Nitisastro and others added 3 commits September 6, 2023 16:57

Merge branch 'master' of github.com:ray-project/ray into fractional_r…

6d69971

…esource_doc

non unit distance resource fractional value fix

eba2d75

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

Merge branch 'master' of github.com:ray-project/ray into fractional_r…

f485d3f

…esource_doc

jonathan-anyscale changed the title ~~[docs] Add small note on documentation for fragmented resource~~ [Core] Non Unit Instance fractional value fix Sep 8, 2023

jonathan-anyscale requested a review from rkooo567 September 9, 2023 01:54

update doc

3b3dcf3

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

rkooo567 self-assigned this Sep 11, 2023

krfricke approved these changes Sep 12, 2023

View reviewed changes

rkooo567 reviewed Sep 12, 2023

View reviewed changes

rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Sep 12, 2023

update more docs

f90109e

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

jonathan-anyscale force-pushed the fractional_resource_doc branch from 18927a6 to f90109e Compare September 13, 2023 23:26

jonathan-anyscale requested a review from rkooo567 September 13, 2023 23:30

jonathan-anyscale removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Sep 13, 2023

rkooo567 approved these changes Sep 15, 2023

View reviewed changes

rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Sep 15, 2023

fix lint

14bc2a1

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

angelinalg approved these changes Sep 15, 2023

View reviewed changes

doc/source/ray-core/scheduling/resources.rst Outdated Show resolved Hide resolved

doc/source/ray-core/scheduling/resources.rst Outdated Show resolved Hide resolved

doc/source/ray-core/scheduling/resources.rst Outdated Show resolved Hide resolved

Update doc/source/ray-core/scheduling/resources.rst

d471d0d

Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com> Signed-off-by: jonathan-anyscale <144177685+jonathan-anyscale@users.noreply.github.com>

jonathan-anyscale and others added 2 commits September 15, 2023 12:31

Update doc/source/ray-core/scheduling/resources.rst

cb9b173

Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com> Signed-off-by: jonathan-anyscale <144177685+jonathan-anyscale@users.noreply.github.com>

Update doc/source/ray-core/scheduling/resources.rst

36763d9

Co-authored-by: angelinalg <122562471+angelinalg@users.noreply.github.com> Signed-off-by: jonathan-anyscale <144177685+jonathan-anyscale@users.noreply.github.com>

jonathan-anyscale removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Sep 15, 2023

rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Sep 21, 2023

merge conflict

947f04a

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

jonathan-anyscale removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Sep 21, 2023

jjyao reviewed Sep 24, 2023

View reviewed changes

jonathan-anyscale added 2 commits September 24, 2023 20:21

cython: split resources str

edc06ab

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

update doc

0516fcc

Signed-off-by: Jonathan Nitisastro <jonathancn@anyscale.com>

jjyao approved these changes Sep 26, 2023

View reviewed changes

doc/source/ray-core/scheduling/resources.rst Outdated Show resolved Hide resolved

Update doc/source/ray-core/scheduling/resources.rst

fe67907

Co-authored-by: Jiajun Yao <jeromeyjj@gmail.com> Signed-off-by: jonathan-anyscale <144177685+jonathan-anyscale@users.noreply.github.com>

jjyao merged commit 54901bf into master Sep 26, 2023
82 of 85 checks passed

jjyao deleted the fractional_resource_doc branch September 26, 2023 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] Non Unit Instance fractional value fix #39293

[Core] Non Unit Instance fractional value fix #39293

jonathan-anyscale commented Sep 6, 2023 •

edited

jjyao left a comment

jjyao Sep 6, 2023

krfricke left a comment

rkooo567 Sep 12, 2023

rkooo567 Sep 12, 2023

rkooo567 Sep 12, 2023

rkooo567 commented Sep 15, 2023

angelinalg left a comment

rkooo567 commented Sep 21, 2023

jjyao Sep 24, 2023

jjyao Sep 24, 2023

jonathan-anyscale Sep 25, 2023

jjyao Sep 24, 2023

jjyao Sep 24, 2023

	Fractional input must be between 0 to 1. For any value greater than 1, `num_cpus` and `num_gpus` requires a whole number.
	Fractional resource requirement must be smaller than 1. For any resource requirement greater than 1, it needs to be a whole number (e.g. ``num_cpus=1.5`` is invalid).

	test_frac_cpu.remote()
	assert ray.get(test_frac_cpu.remote())

		unit_resources = f"{RayConfig.instance().predefined_unit_instance_resources()},\
		{RayConfig.instance().custom_unit_instance_resources()}"

		"Unit instance resource (GPU, TPU, Neuron Core) quantities >1 must",
		f" be whole numbers. {key} resource with value {value} is invalid.")

[Core] Non Unit Instance fractional value fix #39293

[Core] Non Unit Instance fractional value fix #39293

Conversation

jonathan-anyscale commented Sep 6, 2023 • edited

Why are these changes needed?

Related issue number

Checks

jjyao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krfricke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rkooo567 commented Sep 15, 2023

angelinalg left a comment

Choose a reason for hiding this comment

rkooo567 commented Sep 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathan-anyscale commented Sep 6, 2023 •

edited