"logarthmic" to "logarithmic", improve readability, test 2D patterns and optimal IB

[chriscoey]

• change "logarthmic" to "logarithmic"
• slightly improve readability/consistency, especially of tests
• test 2D patterns
• test optimal IB scheme (tragically slow)

[coveralls]

Coverage increased (+21.8%) to 77.635% when pulling d9881f0 on chriscoey:cc_cleanup into 184ea70 on joehuchette:master.

[codecov-io]

Codecov Report

Merging #25 into master will increase coverage by 21.84%.
The diff coverage is 62.18%.

@@             Coverage Diff             @@
##           master      #25       +/-   ##
===========================================
+ Coverage   55.79%   77.63%   +21.84%     
===========================================
  Files           2        2               
  Lines         708      702        -6     
===========================================
+ Hits          395      545      +150     
+ Misses        313      157      -156

Impacted Files	Coverage Δ
src/types.jl	89.7% <28.57%> (+36.76%)	⬆️
src/jump.jl	76.34% <63.05%> (+20.24%)	⬆️

---

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 184ea70...d9881f0. Read the comment docs.

[chriscoey]

@joehuchette optimal IB scheme is extremely slow to calculate (even for 4 breakpoints).. is it ever worthwhile?

also, where do the values for z in your tests come from? they don't match the true function values at the points, though they are the correct optimal solutions for the models constructed. are you just checking for consistency across the different formulations?

[chriscoey]

if optimal IB scheme test takes too long on travis, I will remove it. quite absurd how slow it is.

edit: reduced it to 3x3 breakpoints - works ok

[chriscoey]

I'm satisfied at this point

[joehuchette]

Looks fine to me, but the diff for the triangulation formulations is hard to read. Did you change that code?

[joehuchette]

Why is this commented out?

[chriscoey]

I commented out 3 parts of code in src/jump.jl:

• sos2 MC formulation
• optimal triangle selection pattern
• stencil pattern

because they all appeared to me to be not useable/accessible. imo it's better (and less confusing) to have untestable dead code commented out.

the two patterns above are not recognized in BivariatePWLFunction so it fails without an error message on https://github.com/joehuchette/PiecewiseLinearOpt.jl/blob/184ea708cd3d3869a95cd62b9dcb1923d03a1a77/src/types.jl#L101

[chriscoey]

I only made a few small aesthetic changes, mostly to comments, in the triangulation formulations

[joehuchette]

They're accessible if you change the pattern field after construction; I had research code that used this. The interface to specify the formulation needs to be rewritten (on my TODO list), but the code is not dead and shouldn't be removed.

[chriscoey]

OK so it sounds like a simple fix to allow the two extra patterns, right? a few extra lines in BivariatePWLFunction. we can do it here and then cover the two patterns with unit tests.

which pattern did you need to specify before you could later change the pattern to stencil? and same question for triangle selection.

[joehuchette]

Yes, it should be a simple change to support them "officially".

I would generate the triangulation randomly, then change the pattern field manually. The conflation here between structure (random) and formulation (e.g. optimal triangle selection) is not great, and one of the reasons I would like to change the interface.

[chriscoey]

OK I uncommented. can you open an issue for the expected interface change

[joehuchette]

Thanks! #28 is for the interface change, take a look and let me know your thoughts.