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

use distributed processing from DiDa #203

Merged
merged 5 commits into from
Mar 29, 2022
Merged

use distributed processing from DiDa #203

merged 5 commits into from
Mar 29, 2022

Conversation

exaexa
Copy link
Collaborator

@exaexa exaexa commented Mar 19, 2022

After the performance fixes in DistributedData (JuliaLang/julia#44671 (comment), LCSB-BioCore/DistributedData.jl#40 ) it no longer seems vital to maintain a custom clone of the functionality in GigaSOM. This switches to all functions from DistributedData.

In the process of cleaning I also axed visualizations/ as they were not even included in the code (:rofl: ) and bumped to 0.7 so that people may start depending on this.

There might be various leftovers from original gigasom-internal DiDa, esp in docs. I checked a lot with sed but still kinda betting on quality reviews. :D

Copy link
Member

@laurentheirendt laurentheirendt left a comment

Choose a reason for hiding this comment

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

Difficult to check every line of it, but looks good as far as I can judge

@laurentheirendt
Copy link
Member

Why is CodeCov not responding? Would be interesting to check the testing coverage in this case

@exaexa
Copy link
Collaborator Author

exaexa commented Mar 21, 2022

I want Oliver to actually check this in his settings, let's merge after he acks that it works for him.

Why is CodeCov not responding? Would be interesting to check the testing coverage in this case

good question, I've got no idea why it works as it works tbh. Maybe it doesn't care about merging to develop?

..............let's make our own with Julia'n'stuff? :D

@codecov
Copy link

codecov bot commented Mar 22, 2022

Codecov Report

Merging #203 (567ceff) into develop (42af827) will increase coverage by 2.29%.
The diff coverage is 97.95%.

@@             Coverage Diff             @@
##           develop     #203      +/-   ##
===========================================
+ Coverage    91.47%   93.76%   +2.29%     
===========================================
  Files           11        8       -3     
  Lines          598      385     -213     
===========================================
- Hits           547      361     -186     
+ Misses          51       24      -27     
Impacted Files Coverage Δ
src/base/trainutils.jl 95.12% <0.00%> (+0.25%) ⬆️
src/analysis/core.jl 94.73% <100.00%> (+0.21%) ⬆️
src/analysis/embedding.jl 95.60% <100.00%> (ø)
src/base/dataops.jl 100.00% <100.00%> (+2.10%) ⬆️
src/base/structs.jl 100.00% <100.00%> (ø)
src/io/input.jl 91.13% <100.00%> (+0.11%) ⬆️
src/io/process.jl 87.93% <100.00%> (ø)
src/io/splitting.jl 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b784d10...567ceff. Read the comment docs.

@laurentheirendt
Copy link
Member

Great - thx for fixing Codecov!

@exaexa
Copy link
Collaborator Author

exaexa commented Mar 29, 2022

So this actually seems to work just right on ULHPC so I'm going to fastforward a bit :D @oHunewald lmk in case you hit any issues.

@exaexa exaexa merged commit d44213f into develop Mar 29, 2022
@exaexa exaexa deleted the mk-dida branch March 29, 2022 19:49
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