search

convexengineering / gpfit

Fit posynomials to data
http://gpfit.readthedocs.io/en/latest/
MIT License
10 stars 7 forks source link

raise error if fit has a 0 or inf #70

Closed mjburton11 closed 7 years ago

mjburton11 commented 7 years ago

@bqpd and @whoburg ready for review. Sometimes the resulting fit contains a 0 or inf value. I think it should return an error if this happens.

bqpd commented 7 years ago

Looks great! Any clear way to reduce the number of lines of code? Right now it's net +7, not counting the new errors.

mjburton11 commented 7 years ago

I tried reducing it by creating cs and exps from lhs and rhs but ran into some issues with higher dimensions. Any ideas?

bqpd commented 7 years ago

If you use .min() it'll search across any number of dimensions.

mjburton11 commented 7 years ago

@bqpd @galbramc why did it only fail on reynolds?

bqpd commented 7 years ago

It had an extra large RMS error? Non-determinism? different versions of numpy interpreting the same SEED differently? Try running it again.

(I am not a huge fan of this unit-testing setup)

mjburton11 commented 7 years ago

Test this please

mjburton11 commented 7 years ago

Test this please

mjburton11 commented 7 years ago

@bqpd can we set a seed so that it will be more deterministic or something?

bqpd commented 7 years ago

I think we do! This seems fairly consistent though, so maybe it's a real bug?

mjburton11 commented 7 years ago

How can I figure out the bug? I don't think I changed the way the rms is calculated so I don't know why it would be giving different answers.

bqpd commented 7 years ago

Ack, I'll take a look tomorrow. These tests were by far the most frustrating part of refactoring fit

bqpd commented 7 years ago

it passes on my computer, frustrating.

mjburton11 commented 7 years ago

wow, very frustrating...

bqpd commented 7 years ago

Yeah, my best guess is that a different numpy version is interpreting the fixed seed differently. I increased tolerance a bit (from 5e-4 to 1e-3), it's still a darn low error.

mjburton11 commented 7 years ago

OK well I'm going to go ahead and merge this then

bqpd commented 7 years ago

woot!