Skip to content

Comments

[SYSTEMDS-2747] Federated Als-CG Test#1170

Closed
ywcb00 wants to merge 4 commits intoapache:masterfrom
ywcb00:test/fed/alscg
Closed

[SYSTEMDS-2747] Federated Als-CG Test#1170
ywcb00 wants to merge 4 commits intoapache:masterfrom
ywcb00:test/fed/alscg

Conversation

@ywcb00
Copy link
Contributor

@ywcb00 ywcb00 commented Jan 24, 2021

Hi,
This is a PR for adding federated Als-CG as a first algorithm-level test for federated quaternary operations.
I also added a branch inside FEDInstructionUtils for rewriting BinarySP instructions into BinaryFED instructions in order to support the "fed_!=" instruction needed inside the als-CG algorithm. Since I haven't found a test for these federated operations, I added a test for MatrixScalar and MatrixMatrix federated logical operations.
I made a small fix (I hope it was a fix xD) inside BinaryMatrixMatrixFEDInstruction by changing a broadcast() to a broadcastSliced().
Hope that was not too much, otherwise just revert the last commit (bce7fdc) and the AlsCG test is using "sp_!=" for spark execution mode again.

Thanks for review :)

@ywcb00
Copy link
Contributor Author

ywcb00 commented Jan 26, 2021

I've made a new commit (8c69017) to revert the changes of BinaryMatrixMatrixFEDInstruction.java because some federated algorithm tests were failing. In addition, I ignored the MatrixMatrix tests of the new FederatedLogicalTest (not working without the previous change) while still keeping the MatrixScalar tests (still working).

…struction in checkAndReplaceSP()

fix(BinaryMatrixMatrixFEDInstruction.java): change broadcast of mo2 to broadcast sliced

feat(FedLogical): add tests for federated logical MatrixScalar and MatrixMatrix instructions

fix(alsCG): add check for heavy hitter "fed_!=" - now supported for SPARK too :)
…dcasting sliced

chore(fedLogical): ignore the MatrixMatrix tests - keep the MatrixScalar Tests
…t is a matrix and row partitioned

	distinguish the case where mo2 is a column vector --> don't broadcast slice even if it is row partioned

chore(FedLogical): remove ignores of MatrixMatrix tests
@ywcb00
Copy link
Contributor Author

ywcb00 commented Jan 27, 2021

Found my mistake - we need to distinguish between column vector (normal broadcast) and matrix (broadcast sliced if row partitioned) in BinaryMatrixMatrixFEDInstruction.java.
Commit 8059c5e

@mboehm7
Copy link
Contributor

mboehm7 commented Jan 30, 2021

LGTM - thanks @ywcb00. I just disabled the spark tests for ALS because forced spark exec mode hurts here in terms of execution (repeated lazy evaluation as the no intermediates are materialized).

As we discussed offline, please go ahead and do an additional cleanup over the federated binary element-wise operations to clearly differentiate between row and column partitioned federated data for matrix-row vector, matrix-column vector, and matrix-matrix. Thanks.

@asfgit asfgit closed this in 65d6ad3 Jan 30, 2021
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