Skip to content

remove passing of size to stream clone and repack#1306

Closed
jmitrevs wants to merge 7 commits into
fastmachinelearning:mainfrom
jmitrevs:oneAPI_clone_stream
Closed

remove passing of size to stream clone and repack#1306
jmitrevs wants to merge 7 commits into
fastmachinelearning:mainfrom
jmitrevs:oneAPI_clone_stream

Conversation

@jmitrevs

@jmitrevs jmitrevs commented Jun 6, 2025

Copy link
Copy Markdown
Contributor

Description

This removes the unused feature of clone stream and repack for oneAPI. Generally with oneAPI we don't support reading in multiple inputs as one.

While looking at the changes I found the bug: stream6 was written twice, with the data of stream 6 and stream7, while no data was sent to stream7. That fix has been incorporated here. It is easy to incorporate that fix in the master branch directly, but I think these other changes are worth having.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • Other - remove unused feature

Tests

The bug fix should make test_multi_fix work for oneAPI, and other pytests should not break because of the other changes.

Checklist

  • I have read the guidelines for contributing.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have installed and run pre-commit on the files I edited or added.
  • I have added tests that prove my fix is effective or that my feature works.

@jmitrevs jmitrevs added the please test Trigger testing by creating local PR branch label Jun 6, 2025
@jmitrevs jmitrevs marked this pull request as ready for review July 28, 2025 22:33
@jmitrevs jmitrevs added bug please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Jul 28, 2025
@jmitrevs jmitrevs added this to the v1.2.0 milestone Jul 28, 2025
@jmitrevs jmitrevs added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Jul 30, 2025
@JanFSchulte

Copy link
Copy Markdown
Contributor

The latest changes improved things and seem to have actually fixed the stuck stream cloning test. But there are 2 remaining stuck tests, one in the ``test_pytorch_apitest and one intest_transpose_concat`. Also, now that the stream clone test pass, it reveals that all tests of `test_time_distributed` fail, but that is unrelated to this PR.

@jmitrevs

Copy link
Copy Markdown
Contributor Author

The test_view test seems to hang. Will investigate.

@jmitrevs jmitrevs added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Aug 1, 2025
@jmitrevs

jmitrevs commented Aug 4, 2025

Copy link
Copy Markdown
Contributor Author

It turns out we do need to pass the size after all. The fixes discovered when working on this PR are now in #1354.

@jmitrevs jmitrevs closed this Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug please test Trigger testing by creating local PR branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants