Perf

[mtanneau]

Basically a close-to-full re-write, with major performance improvements (see below).
The diff will be horrible, since virtually no line was left untouched.

The main differences are as follows:

• Instead of parsing each section in a separate function (with lots of duplicate code), I introduced the MPSCard structure for extracting fields. Not all fields are always strings or always numbers, so only strings are extracted.
  I then replaced read_XXX_section with read_XXX_line, which only handles the parsing of fields, which shortens the functions and removes a lot of duplicate code.
  This approach should make it (much) easier to support Free MPS format, since only the read_card! function will have to be modified
• Strings are converted to integers whenever possible (e.g., for handling section headers), and hashes are computed as rarely as possible.
• The QPSData object is created at the beginning of readqps, and modified in-place. This saves a lot of memory

Other minors differences:

• Error messages now display the line number
• No error is thrown if the RANGES section is read before the RHS section. This is because, in absence of an RHS section, all right-hand sides are assumed to be zero.
  However, an error is thrown if RANGES appears before RHS.
• I removed the @warn for the first time a rim field is detected. Instead, a @error log is displayed every time such data is encountered. Tests were updated accordingly.

Compared to the master branch, ~75% decrease in time, and ~85% decrease in memory:

ID	time ratio	memory ratio
["maros"]	0.27 (5%) ✅	0.17 (1%) ✅
["netlib"]	0.25 (5%) ✅	0.15 (1%) ✅

Other solvers do not read .SIF files, so explicit comparison is not possible on this testset.
Nevertheless, reading all NETLIB instances (which I manually converted to MPS) with Gurobi (through its Julia API) takes ~1.5s, compared to ~6s for QPSReader.

[dpo]

Do we need to phase out julia < 1.3 here?

[mtanneau]

We may not have to.

Some tests fail because I sometimes use isnothing(x) instead of x === nothing. This function only exists for Julia1.1 and later.

What I understand less well is why

qp = @test_logs(
        # These logs must appear in exactly this order
        (:info, "Problem name     : QP example"),
        (:info, "Objective sense  : notset"),
        (:info, "Objective name   : obj"),
        (:info, "RHS              : rhs1"),
        (:info, "RANGES           : nothing"),
        (:info, "BOUNDS           : bnd1"),
        match_mode = :all,
        readqps("dat/qp-example.qps")
    )

results in qp being nothing in Julia1.0, and qp being a QPSData in Julia1.3

[mtanneau]

Looks like the culprit is

ranges_section_read && @info("RANGES           : $(qpsdat.rngname)")

The following will error in Julia1.0 but not in 1.3:

x = nothing
@info "$x"

This can happen if the RANGES section is empty, in which case ranges_section_read is true but qps.rngname is nothing.

Two remedies:

• Remove these logs (which means less information will be displayed
• Log each individually during the reading. For instance,

if qps.rngname === nothing
    # Record this as the RANGES
    qps.rngname = rng
    @info "Using $rng at line $(card.line) as RANGES name"

[codecov]

Codecov Report

Merging #20 into master will decrease coverage by 0.81%.
The diff coverage is 76.42%.

@@            Coverage Diff            @@
##           master     #20      +/-   ##
=========================================
- Coverage   78.32%   77.5%   -0.82%     
=========================================
  Files           2       2              
  Lines         429     369      -60     
=========================================
- Hits          336     286      -50     
+ Misses         93      83      -10

Impacted Files	Coverage Δ
src/readqps.jl	77.44% <76.42%> (-0.83%)	⬇️

---

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 1bf2b8b...ae521df. Read the comment docs.

[dpo]

I'm happy to drop Julia < 1.3.

[mtanneau]

The latest commit works on Julia 1.0 - 1.4 (latter tested locally), so dropping < 1.3 is not necessary (and 1.0 is long-term supported).

That can always be changed later

[mtanneau]

Do we need something else before this can be merged?

[dpo]

No, thanks a lot. Sorry for the latency.