Add more compression formats #90

dourouc05 · 2019-10-30T01:15:57Z

Following jump-dev/JuMP.jl#1982, here is the implementation of two other compression formats (BZIP2 and XZ). While I was at it, I also refactored a bit the common functions (in the hope that adding new compression formats and new export formats will be easier, the changes to be done being centralised).

For now, I did not add tests, as I'm not sure which ones I should add (currently, it seems compression is handled within each file format, but I think just testing one of them should be enough -- e.g., just LP, as nothing is really dependent on the file format).

…ion codecs.

codecov-io · 2019-10-30T01:20:30Z

Codecov Report

Merging #90 into master will decrease coverage by 0.03%.
The diff coverage is 97.22%.

@@            Coverage Diff             @@
##           master      #90      +/-   ##
==========================================
- Coverage   95.92%   95.88%   -0.04%     
==========================================
  Files           8        9       +1     
  Lines        1275     1289      +14     
==========================================
+ Hits         1223     1236      +13     
- Misses         52       53       +1

Impacted Files	Coverage Δ
src/compression.jl	`100% <100%> (ø)`
src/MOF/MOF.jl	`100% <100%> (ø)`	⬆️
src/MathOptFormat.jl	`95.08% <91.66%> (-2.07%)`	⬇️

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 cc7e817...2b71875. Read the comment docs.

odow · 2019-10-30T15:43:41Z

Instead of the enum's for compression, I wonder if a better approach is to define an abstract type so that people can add different compression schemes without having to depend on a whole lot of different packages.

abstract type AbstractCompressionScheme end

struct NoCompression <: AbstractCompressionScheme end
function compressed_open(
    f::Function, filename::String, mode::String, ::NoCompression
) 
    return open(f, filename, mode)
end

struct Gzip <: AbstractCompressionScheme end
function open(
    f::Function, filename::String, mode::String, ::Gzip
)
    if mode == "w"
        open(f, CodecZlib.GzipCompressorStream, filename, "w")
    else
        ...
    end
end

dourouc05 · 2019-10-30T21:13:58Z

I'm trying to implement your approach, with types. However, when looping over the compression formats (detect the format from the extension), I don't see any clean way of implementing it with types. I find that subtypes is not really clean, but I cannot see anything else…

Here is what I did right now: 698b8cc

odow · 2019-10-30T21:33:13Z

Just force the user to provide the compression scheme. Define

function read_from_file(filename; compression_scheme = nothing)
    if compression_scheme === nothing
        compression_scheme = endswith(filename, ".gz") ? Gzip() : NoCompression()
    end
    compressed_open(filename, "r", compression_scheme) do
        read_from_file(...)
    end
end

dourouc05 · 2019-10-30T23:21:01Z

Better this way? I still define three "default" compression schemes, supported everywhere. The latest commit also includes a little bit of documentation.

Should the new open methods overload Base.open? Right now, if I understood correctly what's happening, it seems like there is a new function in MOF (with one method per compression scheme). I had to do things like return Base.open to make it work.

odow · 2019-10-31T14:31:00Z

src/MathOptFormat.jl

+    return _file_formats[_filename_to_format(filename)][2]()
+end
+
+function gzip_open(f::Function, filename::String, mode::String; compression::AbstractCompressionScheme=AutomaticCompressionDetection())


Remove this function.

odow · 2019-10-31T14:34:13Z

src/compression.jl

+abstract type AbstractCompressionScheme end
+
+struct AutomaticCompressionDetection <: AbstractCompressionScheme end
+# No open() implementation, this would not make sense (flag to indicate that _filename_to_compression should be called).


function _compressed_open( f::Function, filename::String, mode::String, ::AutomaticCompression ) compression = _filename_to_compression(filename) return _compressed_open(f, filename, mode, compression) end

odow · 2019-10-31T14:34:22Z

src/compression.jl

+# No open() implementation, this would not make sense (flag to indicate that _filename_to_compression should be called).
+
+struct NoCompression <: AbstractCompressionScheme end
+function open(


_compressed_open

odow · 2019-10-31T14:34:30Z

src/compression.jl

+end
+
+struct Gzip <: AbstractCompressionScheme end
+function open(


_compressed_open

odow · 2019-10-31T14:34:39Z

src/compression.jl

+end
+
+struct Bzip2 <: AbstractCompressionScheme end
+function open(


_compressed_open

odow · 2019-10-31T14:34:46Z

src/compression.jl

+end
+
+struct Xz <: AbstractCompressionScheme end
+function open(


_compressed_open

odow · 2019-10-31T14:35:09Z

src/compression.jl

+"""
+abstract type AbstractCompressionScheme end
+
+struct AutomaticCompressionDetection <: AbstractCompressionScheme end


Just AutomaticCompression

odow · 2019-10-31T14:36:04Z

src/MathOptFormat.jl

-function MOI.write_to_file(model::MATH_OPT_FORMATS, filename::String)
-    gzip_open(filename, "w") do io
+function MOI.write_to_file(model::MATH_OPT_FORMATS, filename::String; compression::AbstractCompressionScheme=AutomaticCompressionDetection())
+    gzip_open(filename, "w", compression=compression) do io


_compressed_open(filename, "w", compression) do io

odow · 2019-10-31T14:36:19Z

src/MathOptFormat.jl

-function MOI.read_from_file(model::MATH_OPT_FORMATS, filename::String)
-    gzip_open(filename, "r") do io
+function MOI.read_from_file(model::MATH_OPT_FORMATS, filename::String; compression::AbstractCompressionScheme=AutomaticCompressionDetection())
+    gzip_open(filename, "r", compression=compression) do io


_compressed_open(filename, "r", compression) do io

odow · 2019-10-31T14:53:14Z

src/MathOptFormat.jl

-    else
-        error("File-type of $(filename) not supported by MathOptFormat.jl.")
-    end
+    model = _filename_to_model(filename)


function read_from_file( filename::String; compression::AbstractCompressionScheme = AutomaticCompression(), file_format::FileFormat = AUTOMATIC_FILE_FORMAT, ) if file_format == AUTOMATIC_FILE_FORMAT for (format, (ext, _)) in _FILE_FORMATS if endswith(filename, ext) || occursin("$(ext).", filename) file_format = format break end end error( "Unable to detect automatically format of $(filename). Use the " * "`file_format` keyword to specify the file format." ) end model = _FILE_FORMATS[file_format]() MOI.read_from_file(model, filename; compression = compression) return model end

You might want to do something similar with AutomaticCompression so that we don't try NoCompression if the user passes in a weird filename.

dourouc05 · 2019-11-01T04:30:56Z

Better this way?

dourouc05 · 2019-11-02T23:12:48Z

I've added a few tests. One of them fails, I registered an issue for that (JuliaIO/CodecXz.jl#16).

odow · 2019-11-03T14:19:46Z

Just remove Xz for now

dourouc05 · 2019-11-03T14:56:31Z

Shall I remove the code or comment it out in this PR?

…

On Sun, 3 Nov 2019, 15:19 Oscar Dowson, ***@***.***> wrote: Just remove Xz for now — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#90?email_source=notifications&email_token=AACFY6QR7TF6D3UMK5QJU6DQR3FXHA5CNFSM4JGR5CK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5TUNQ#issuecomment-549141046>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACFY6UYUMEGL3HAJ2PHGILQR3FXHANCNFSM4JGR5CKQ> .

dourouc05 · 2019-11-08T14:49:17Z

I've commented out this code so it can easily be restored once the package is fixed.

Project.toml

dourouc05 · 2019-11-26T15:53:54Z

All tests pass; could this be merged?

odow · 2019-11-26T16:13:58Z

Sorry for moving slowly on this. Thanks!

dourouc05 · 2019-11-26T16:14:12Z

No problem, thanks for merging :)!

dourouc05 added 3 commits October 30, 2019 01:42

Move most of the code from jump-dev/JuMP.jl#1982.

255664b

Refactor to use a dictionary storing all the variance within compress…

86824a7

…ion codecs.

Same refactoring for model types.

bbd1eed

dourouc05 mentioned this pull request Oct 30, 2019

[WIP] Implementation of write_to_file that use MathOptFormat. jump-dev/JuMP.jl#1982

Closed

Start restructuring compression format handling.

698b8cc

dourouc05 added 2 commits October 31, 2019 00:03

Finalise implementation based on types.

7460eae

Add documentation for AbstractCompressionScheme.

68f635d

odow reviewed Oct 31, 2019

View reviewed changes

Clean up code.

3139964

Add more tests.

38534cf

Comment out XZ.

e7c932d

odow reviewed Nov 8, 2019

View reviewed changes

Project.toml Outdated Show resolved Hide resolved

dourouc05 and others added 2 commits November 8, 2019 16:24

Remove dependency for commented out code.

21cb685

Comment out XZ.

2b71875

odow merged commit a8b6900 into odow:master Nov 26, 2019

dourouc05 deleted the dourouc05-compression branch November 26, 2019 16:14

Add more compression formats #90

Add more compression formats #90

Uh oh!

Conversation

dourouc05 commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

odow commented Oct 30, 2019

Uh oh!

dourouc05 commented Oct 30, 2019

Uh oh!

odow commented Oct 30, 2019

Uh oh!

dourouc05 commented Oct 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dourouc05 commented Nov 1, 2019

Uh oh!

dourouc05 commented Nov 2, 2019

Uh oh!

odow commented Nov 3, 2019

Uh oh!

dourouc05 commented Nov 3, 2019 via email

Uh oh!

dourouc05 commented Nov 8, 2019

Uh oh!

Uh oh!

dourouc05 commented Nov 26, 2019

Uh oh!

odow commented Nov 26, 2019

Uh oh!

dourouc05 commented Nov 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dourouc05 commented Oct 30, 2019 •

edited

Loading

codecov-io commented Oct 30, 2019 •

edited

Loading