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

Add Faust Dataset #160

Merged
merged 12 commits into from Sep 1, 2022
Merged

Add Faust Dataset #160

merged 12 commits into from Sep 1, 2022

Conversation

Dsantra92
Copy link
Collaborator

Closes #155

@Dsantra92 Dsantra92 marked this pull request as ready for review July 25, 2022 14:37
@Dsantra92
Copy link
Collaborator Author

The unit test is causing problems because it cannot download from UCI archives.

src/datasets/meshes/mpi_faust.jl Outdated Show resolved Hide resolved
src/datasets/meshes/mpi_faust.jl Outdated Show resolved Hide resolved
src/datasets/meshes/mpi_faust.jl Outdated Show resolved Hide resolved
src/datasets/meshes/mpi_faust.jl Outdated Show resolved Hide resolved
src/datasets/meshes/mpi_faust.jl Outdated Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
src/datasets/meshes/faust.jl Show resolved Hide resolved
src/datasets/meshes/faust.jl Outdated Show resolved Hide resolved
src/datasets/meshes/faust.jl Show resolved Hide resolved
src/datasets/meshes/faust.jl Outdated Show resolved Hide resolved
src/datasets/meshes/faust.jl Outdated Show resolved Hide resolved
@Dsantra92
Copy link
Collaborator Author

@CarloLucibello should we remove the failing changes? I believe the CI downloading dataset is most likely flagged as bot download.

@CarloLucibello
Copy link
Member

I'm not sure why we have an error with the PTBLM dataset that we didn't have before, but we can remove it from the CI tests and open a corresponding issue tracking the problem

1 similar comment
@CarloLucibello
Copy link
Member

I'm not sure why we have an error with the PTBLM dataset that we didn't have before, but we can remove it from the CI tests and open a corresponding issue tracking the problem

@Dsantra92
Copy link
Collaborator Author

I'm not sure why we have an error with the PTBLM dataset that we didn't have before, but we can remove it from the CI tests and open a corresponding issue tracking the problem

Cool

src/datasets/meshes/faust.jl Show resolved Hide resolved
src/datasets/meshes/faust.jl Show resolved Hide resolved
src/datasets/meshes/faust.jl Show resolved Hide resolved
src/datasets/meshes/faust.jl Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2022

Codecov Report

Merging #160 (eb0108d) into master (66edbf7) will decrease coverage by 1.59%.
The diff coverage is 10.34%.

@@            Coverage Diff             @@
##           master     #160      +/-   ##
==========================================
- Coverage   33.62%   32.03%   -1.60%     
==========================================
  Files          40       42       +2     
  Lines        2028     2132     +104     
==========================================
+ Hits          682      683       +1     
- Misses       1346     1449     +103     
Impacted Files Coverage Δ
src/datasets/meshes/faust.jl 2.17% <2.17%> (ø)
src/abstract_datasets.jl 21.87% <22.22%> (-0.71%) ⬇️
src/MLDatasets.jl 100.00% <100.00%> (ø)
src/datasets/text/smsspamcollection.jl 7.69% <0.00%> (-44.16%) ⬇️
src/datasets/graphs/polblogs.jl 84.21% <0.00%> (-5.27%) ⬇️
src/datasets/vision/emnist.jl 5.00% <0.00%> (-5.00%) ⬇️
src/datasets/graphs/reddit.jl 2.12% <0.00%> (-2.13%) ⬇️
src/datasets/graphs/tudataset.jl 1.36% <0.00%> (-1.37%) ⬇️
src/datasets/vision/cifar10.jl 1.20% <0.00%> (-1.21%) ⬇️
src/datasets/vision/cifar100.jl 1.20% <0.00%> (-1.21%) ⬇️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@CarloLucibello
Copy link
Member

the 1.6 test failure seems real. Not sure which escape sequence is invalid on julia 1.6 though

@Dsantra92
Copy link
Collaborator Author

Dsantra92 commented Aug 2, 2022

The problem is backslashes in lines 36-42 breaking two lines. Breaklines compatibility changed in 1.7 as understood from this discourse thread. It would be better to just use longer lines for now to let all the tests pass.

@CarloLucibello
Copy link
Member

The problem is backslashes in lines 36-42 breaking two lines. Breaklines compatibility changed in 1.7 as understood from this discourse thread. It would be better to just use longer lines for now to let all the tests pass.

can we do this and complete this PR?

@CarloLucibello CarloLucibello merged commit e8ec56e into JuliaML:master Sep 1, 2022
@Dsantra92
Copy link
Collaborator Author

Version Bump for this :0

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.

MPI Faust dataset
3 participants