Skip to content

[fix] fix sample type non numeric#966

Closed
guapisolo wants to merge 1 commit intomainfrom
ci/sample_type
Closed

[fix] fix sample type non numeric#966
guapisolo wants to merge 1 commit intomainfrom
ci/sample_type

Conversation

@guapisolo
Copy link
Copy Markdown
Collaborator

@guapisolo guapisolo commented Apr 10, 2026

The default value of sglang weight version is "default" rather than a numeric value. See https://github.com/sgl-project/sglang/blob/e77bfba24d892563fb2d91192e8841b0c59c7828/python/sglang/srt/server_args.py#L435.

When enabled debug rollout only, there is no explicit weight version so it will fail sglang e2e tests.

Fix this CI error. https://github.com/radixark/miles/actions/runs/24220053896/job/70709321587

Traceback (most recent call last):
  File "/__w/miles/miles/train.py", line 109, in <module>
    asyncio.run(train(args))
  File "/usr/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 687, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/__w/miles/miles/train.py", line 73, in train
    rollout_data_ref = await rollout_manager.generate.remote(rollout_id)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ray.exceptions.RayTaskError(ValueError): ray::RolloutManager.generate() (pid=9882, ip=172.18.0.2, actor_id=995f53213b22eb599ff8dc8f02000000, repr=<miles.ray.rollout.RolloutManager object at 0x77e5b77202f0>)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/__w/miles/miles/miles/ray/rollout.py", line 453, in generate
    _log_rollout_data(rollout_id, self.args, data, metrics, time.time() - start_time)
  File "/__w/miles/miles/miles/ray/rollout.py", line 1189, in _log_rollout_data
    log_dict |= dict_add_prefix(compute_metrics_from_samples(args, samples), "rollout/")
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/__w/miles/miles/miles/ray/rollout.py", line 1209, in compute_metrics_from_samples
    oldest_versions = [s.oldest_weight_version for s in samples if s.oldest_weight_version is not None]
                                                                   ^^^^^^^^^^^^^^^^^^^^^^^
  File "/__w/miles/miles/miles/utils/types.py", line 219, in oldest_weight_version
    return min(int(v) for v in self.weight_versions)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/__w/miles/miles/miles/utils/types.py", line 219, in <genexpr>
    return min(int(v) for v in self.weight_versions)
               ^^^^^^
ValueError: invalid literal for int() with base 10: 'default'

@guapisolo guapisolo changed the title [CI] fix sample type [fix] fix sample type non numeric Apr 10, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the oldest_weight_version property to handle non-numeric versions by ignoring them and returning None when no valid integers are found. Unit tests were added to verify this behavior. Feedback was provided to simplify the implementation using a generator expression and the default parameter in the min function to avoid creating intermediate lists.

Comment on lines +223 to +229
versions = []
for version in self.weight_versions:
try:
versions.append(int(version))
except (TypeError, ValueError):
continue
return min(versions) if versions else None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of oldest_weight_version is quite verbose and creates an intermediate list. It can be simplified using a generator expression and the default parameter of the min function for better readability and efficiency.

        numeric_versions = (int(v) for v in self.weight_versions if str(v).isdigit())
        return min(numeric_versions, default=None)

@guapisolo guapisolo closed this Apr 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants