aldro61 / mmit

Regression trees for interval censored output data
https://aldro61.github.io/mmit/
GNU General Public License v3.0
7 stars 7 forks source link

Solver rewritten for (squared) hinge loss #3

Closed aldro61 closed 7 years ago

aldro61 commented 7 years ago

Also includes:

aldro61 commented 7 years ago

@tdhock I was able to isolate a really simple case where the solver fails (see python tests)

aldro61 commented 7 years ago

I found the reason why this test is failing. At some point in the execution, a sign -1 breakpoint is added a few decimals before a sign 1 breakpoint. Because of our floating point tolerance (1e-7), the solver determines that there is no slack at the minimum position and does not update the minimum function. After this, the execution is broken and leads to an incorrect solution.

This is shown below. At the second insertion, the minimum pointer should have been moved, but the slack is so small that it's considered to be 0.

INSERTION START:
----------------------------
The minimum breakpoint is at position inf
The slack at the min breakpoint is inf
Attempting to MOVE LEFT
|_____ moved to breakpoint 0.668536 (Coefficients: (a=0, b=0, c=0))

INSERTION COMPLETED
----------------------------
Inserted y: 1.66854
Breakpoint position: 0.668536
Bound type: upper
N pointer moves: 1
Minimum value: 0
Minimum position: 0.570261
Minimum coefficients: (a=0, b=0, c=0)
The minimum pointer points to the breakpoint at 0.668536
Current breakpoints: {(-0.726126, (a=-1, b=-1.45225, c=-0.527259)), (0.471986, (a=-1, b=0.943973, c=-0.222771)), (0.668536, (a=1, b=-1.33707, c=0.446941)), }

INSERTION START:
----------------------------
The minimum breakpoint is at position 0.668536
The slack at the min breakpoint is 1.51958e-08

INSERTION COMPLETED
----------------------------
Inserted y: -0.33134
Breakpoint position: 0.66866
Bound type: lower
N pointer moves: 0
Minimum value: 0
Minimum position: 0.570261
Minimum coefficients: (a=0, b=0, c=0)
The minimum pointer points to the breakpoint at 0.668536
Current breakpoints: {(-0.726126, (a=-1, b=-1.45225, c=-0.527259)), (0.471986, (a=-1, b=0.943973, c=-0.222771)), (0.668536, (a=1, b=-1.33707, c=0.446941)), (0.66866, (a=-1, b=1.33732, c=-0.447106)), }

I changed this tolerance to 1e-9, but this generates errors in the random tests, which must now be investigated. The problem is that at 1e-9, the floating point comparison functions return incorrect values.

I guess we should keep a low tolerance and modify the map's comparison function to use the same tolerance.

aldro61 commented 7 years ago

All python tests pass on simulated and real data. R tests are failing.

aldro61 commented 7 years ago

All tests passing! We now have a working implementation for the linear/squared hinge losses.

tdhock commented 7 years ago

everything looks good to me. no problem if you want to merge now. the only thing you may want to add before merging would be a couple of trivial tests for the squared hinge loss, with only 2 or 3 data points, where you can compute the optimal loss and predicted value by hand.

aldro61 commented 7 years ago

Thanks for the review. I have some tests for the squared hinge loss where I calculated the solution by hand here: https://github.com/aldro61/mmit/blob/727d781062c60074c16ff7523737c765b79facf9/mmit/tests/test_solver.py#L233