Skip to content

[Build] Add Windows github runner#4

Merged
hughperkins merged 24 commits intomainfrom
hp/windows-runner
Jun 10, 2025
Merged

[Build] Add Windows github runner#4
hughperkins merged 24 commits intomainfrom
hp/windows-runner

Conversation

@hughperkins
Copy link
Copy Markdown
Collaborator

@hughperkins hughperkins commented Jun 2, 2025

Issue: #

Brief Summary

Add Windows github runner, using github standard runner, with windows server core 2025

copilot:summary

Walkthrough

copilot:walkthrough

@hughperkins hughperkins changed the title [Build] Windows runner on demand [Build] Add Windows github runner Jun 2, 2025
Comment thread .github/workflows/win_ondemand.yml Outdated
Comment thread .github/workflows/win_ondemand.yml Outdated
Comment thread tests/python/test_ad_ndarray.py Outdated
Comment thread .github/workflows/scripts_new/win.ps1 Outdated
Comment thread tests/python/test_sparse_matrix.py Outdated
@hughperkins
Copy link
Copy Markdown
Collaborator Author

Note: the markup links wont be fixed until we approve and merge #9 (as well as deciding what to do with that one last link)

Comment thread tests/python/test_ad_ndarray.py Outdated
@hughperkins
Copy link
Copy Markdown
Collaborator Author

Thanks! 🙌

@duburcqa duburcqa self-requested a review June 5, 2025 21:37
@hughperkins
Copy link
Copy Markdown
Collaborator Author

Maybe I should make the update to the test be on a separate PR 🤔

@duburcqa
Copy link
Copy Markdown
Contributor

duburcqa commented Jun 6, 2025

Maybe I should make the update to the test be on a separate PR 🤔

yes that would be nice.

Copy link
Copy Markdown

@CharlesMasson CharlesMasson left a comment

Choose a reason for hiding this comment

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

One comment, LGTM otherwise. 👍

assert x.grad[1] == -0.25


@pytest.mark.flaky(retries=5)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we add comments to explain why those are needed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think it's self documenting? "these tests are flaky. reason unclear"?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes, I understand this part, but I lack context about whether this is expected and we're fine with it, whether someone has additional context about it, whether it's meant to be investigated and fixed, etc.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's not expected, and we're not fine with it, but we don't know how to fix it right now.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

(and there are some much bigger things we want to fix first, like:

  • not having to recompile every time we modify the batch size
  • recompilations not taking 3 minutes
  • start from warm cache not taking 20 seconds)

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.

3 participants