[tune] Treat checkpoints with nan value as worst

[Yard1]

Why are these changes needed?

Changes the logic in CheckpointManager to consider checkpoints with nan value of the metric as worst values, meaning they will be deleted first if keep_checkpoints_num is set.

Related issue number

Closes #23856

Checks

• 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 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 :(

[Yard1]

cc @XuehaiPan

[XuehaiPan]

It needs similar changes with ray/train/checkpoint.py.

[amogkam]

Thanks for the PR @Yard1 and thanks for raising the issue @XuehaiPan!

[amogkam]

Should we use the ml_utils is_nan directly and remove it from tune.utils?

[Yard1]

I think it's fine to use an alias - we have a precedent for that already.

[krfricke]

Generally good, one suggestion

[amogkam]

This looks great @Yard1! Can we resolve the conflicts, and then I can merge!

[XuehaiPan]

When priority is nan, sorting by tuple key:

(not is_nan(priority), priority, checkpoint.order)

won't give the correct order by checkpoint.order. Because both nan < nan and nan > nan return False.

Suggested change

	if self._checkpoint_score_desc:
	priority = -priority
	return (not is_nan(priority), priority, checkpoint.order)
	if self._checkpoint_score_desc:
	priority = -priority
	if is_nan(priority):
	return (0, checkpoint.order, priority)
	return (1, priority, checkpoint.order)

[krfricke]

Actually it does:

>>> import numpy as np
>>> (False, np.nan, 3) < (False, np.nan, 4)
True
>>> (False, np.nan, 4) < (False, np.nan, 3)
False

[XuehaiPan]

Actually it does:

>>> import numpy as np
>>> (False, np.nan, 3) < (False, np.nan, 4)
True
>>> (False, np.nan, 4) < (False, np.nan, 3)
False

Seems that tuple.__lt__ skips items when lhs[i] is rhs[i].

In [1]: (False, float('nan'), 3) < (False, float('nan'), 4)
Out[1]: False

In [2]: (False, float('nan'), 3) > (False, float('nan'), 4)
Out[2]: False

In [3]: (False, float('nan'), 4) < (False, float('nan'), 3)
Out[3]: False

In [4]: (False, float('nan'), 4) > (False, float('nan'), 3)
Out[4]: False

In [5]: import numpy as np

In [6]: (False, np.nan, 3) < (False, np.nan, 4)
Out[6]: True

In [7]: (False, np.nan, 3) > (False, np.nan, 4)
Out[7]: False

In [8]: import math

In [9]: (False, math.nan, 3) < (False, math.nan, 4)
Out[9]: True

In [10]: (False, math.nan, 3) > (False, math.nan, 4)
Out[10]: False

In [11]: float('nan') is float('nan')
Out[11]: False

In [12]: np.nan is np.nan
Out[12]: True

In [13]: math.nan is math.nan
Out[13]: True

In [14]: float('nan') is math.nan
Out[14]: False

In [15]: math.nan is np.nan
Out[15]: False

In [16]: (False, math.nan, 3) < (False, np.nan, 4)
Out[16]: False

np.nan is a single variable, but each call of float('nan') will create a new variable.

[krfricke]

Ah thanks, you're right. It seems indeed like float('nan') <= float('nan') is False, unlike for np.

Fix here: #23909