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

[SYSTEMDS-3729] Fix Federated Response in Roll Function #2131

Closed
wants to merge 1 commit into from

Conversation

min-guk
Copy link
Contributor

@min-guk min-guk commented Oct 21, 2024

I previously wrote the federated roll function to return the result matrix block through a federated response for debugging purposes. Although this does not affect the computation results, it unnecessarily degrades performance, so I will remove it.

Moving forward, I will carefully review the code before committing. I apologize for the inconvenience caused.

Copy link

codecov bot commented Oct 21, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.13%. Comparing base (9b6a96d) to head (406fb89).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
.../runtime/instructions/fed/ReorgFEDInstruction.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2131      +/-   ##
============================================
- Coverage     71.14%   71.13%   -0.01%     
+ Complexity    43048    43047       -1     
============================================
  Files          1446     1446              
  Lines        164246   164246              
  Branches      32018    32018              
============================================
- Hits         116847   116841       -6     
  Misses        38249    38249              
- Partials       9150     9156       +6     

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

@mboehm7
Copy link
Contributor

mboehm7 commented Oct 24, 2024

Thanks for the patch @min-guk and sorry for missing this debugging relict during the merge of the previous PR.

@mboehm7 mboehm7 closed this in 63b99e5 Oct 24, 2024
@min-guk
Copy link
Contributor Author

min-guk commented Oct 25, 2024

@mboehm7 Not at all, I should have been more mindful.

I really appreciate your always kind and insightful review!

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