Skip to content
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

[core] Fix ray.timeline() #36676

Merged
merged 2 commits into from
Jun 22, 2023

Conversation

stephanie-wang
Copy link
Contributor

Why are these changes needed?

Fix a typo bug in ray.timeline() and add a basic test.

Signed-off-by: Stephanie Wang <swang@cs.berkeley.edu>
Signed-off-by: Stephanie Wang <swang@cs.berkeley.edu>
@stephanie-wang
Copy link
Contributor Author

@rickyyx I couldn't figure out how to test the actual content. It looks like this is using a different codepath from the chrome_tracing_dump that's tested in the rest of the file? ray._private.profiling.chrome_tracing_dump vs GlobalState.chrome_tracing_dump. I left a TODO to test this / squash into one codepath?

@rickyyx
Copy link
Contributor

rickyyx commented Jun 21, 2023

@rickyyx I couldn't figure out how to test the actual content. It looks like this is using a different codepath from the chrome_tracing_dump that's tested in the rest of the file? ray._private.profiling.chrome_tracing_dump vs GlobalState.chrome_tracing_dump. I left a TODO to test this / squash into one codepath?

Yeah - I think it was a miss that this wasn't tested when we changed the profiling feature last time. I can stamp this for now while I cleaned up the migration.

@stephanie-wang stephanie-wang merged commit 41311d3 into ray-project:master Jun 22, 2023
@stephanie-wang stephanie-wang deleted the fix-ray-timeline branch June 22, 2023 15:34
arvind-chandra pushed a commit to lmco/ray that referenced this pull request Aug 31, 2023
Fix a typo bug in ray.timeline() and add a basic test.

---------

Signed-off-by: Stephanie Wang <swang@cs.berkeley.edu>
Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants