Skip to content

[SYSTEMDS-3907] Tee OOC operator#2315

Closed
j143 wants to merge 27 commits into
apache:mainfrom
j143:SYSTEMDS-3907-tee-ooc-operator
Closed

[SYSTEMDS-3907] Tee OOC operator#2315
j143 wants to merge 27 commits into
apache:mainfrom
j143:SYSTEMDS-3907-tee-ooc-operator

Conversation

@j143

@j143 j143 commented Aug 24, 2025

Copy link
Copy Markdown
Member

Hi! I tried to rewrite and add tee operator in the graph. The Tee and Transpose tests are working.

Transpose OOC is also implemented (working).

Plan

--MAIN PROGRAM
----GENERIC (lines 23-27) [recompile=false]
------CP createvar pREADX target/testTemp/functions/ooc/TeeTest/in/X false MATRIX binary 1000 1000 1000 1000000 copy
------CP createvar _mVar0 target\testTemp\functions\ooc\TeeTest\Tee/target/scratch_space//_p29756_10.79.14.127//_t0/temp0 true MATRIX binary 1000 1000 1000 1000000 copy
------OOC rblk pREADX.MATRIX.FP64 _mVar0.MATRIX.FP64 1000 true
------CP createvar _mVar1 target\testTemp\functions\ooc\TeeTest\Tee/target/scratch_space//_p29756_10.79.14.127//_t0/temp1 true MATRIX binary 1000 1000 1000 1000000 copy
------OOC tee _mVar0.MATRIX.FP64 tee_out_X_0.MATRIX.FP64 tee_out_X_1.MATRIX.FP64
------CP rmvar _mVar0
------CP createvar _mVar2 target\testTemp\functions\ooc\TeeTest\Tee/target/scratch_space//_p29756_10.79.14.127//_t0/temp2 true MATRIX binary 1000 1000 1000 1000000 copy
------OOC r' tee_out_X_0.MATRIX.FP64 _mVar2.MATRIX.FP64
------CP rmvar tee_out_X_0
------CP createvar _mVar3 target\testTemp\functions\ooc\TeeTest\Tee/target/scratch_space//_p29756_10.79.14.127//_t0/temp3 true MATRIX binary 1000 1000 1000 -1 copy
------OOC ba+* _mVar2.MATRIX.FP64 tee_out_X_1.MATRIX.FP64 _mVar3.MATRIX.FP64 8
------CP rmvar _mVar2 tee_out_X_1
------CP write _mVar3.MATRIX.FP64 target/testTemp/functions/ooc/TeeTest/out/res.SCALAR.STRING.true binary.SCALAR.STRING.true .SCALAR.STRING.true 1000
------CP rmvar _mVar3
------CP cpvar _mVar1 tee_out_X_0
------CP cpvar _mVar1 tee_out_X_1
------CP rmvar _mVar1

Diagram:

image

@j143

j143 commented Aug 27, 2025

Copy link
Copy Markdown
Member Author

Hi @mboehm7 , could you please check this once. thanks.

@codecov

codecov Bot commented Aug 27, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.18605% with 34 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@9e23ad8). Learn more about missing BASE report.
⚠️ Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
...apache/sysds/hops/rewrite/RewriteInjectOOCTee.java 81.92% 6 Missing and 9 partials ⚠️
src/main/java/org/apache/sysds/hops/TeeOp.java 68.75% 10 Missing ⚠️
...ds/runtime/instructions/ooc/TeeOOCInstruction.java 89.18% 4 Missing ⚠️
...time/instructions/ooc/TransposeOOCInstruction.java 87.50% 4 Missing ⚠️
src/main/java/org/apache/sysds/lops/Tee.java 92.85% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2315   +/-   ##
=======================================
  Coverage        ?   72.28%           
  Complexity      ?    46695           
=======================================
  Files           ?     1500           
  Lines           ?   177041           
  Branches        ?    34790           
=======================================
  Hits            ?   127976           
  Misses          ?    39413           
  Partials        ?     9652           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mboehm7

mboehm7 commented Aug 27, 2025

Copy link
Copy Markdown
Contributor

Thanks for the great patch @j143 - overall, this looks very solid. I would recommend to move the TeeOp into the existing DataOp though, and split the OOC transpose into a separate PR (which we can test and merge separately).

@mboehm7 mboehm7 closed this in e51db27 Sep 22, 2025
@github-project-automation github-project-automation Bot moved this from In Progress to Done in SystemDS PR Queue Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants