Half float option

[maosatgithub]

I started a new clean branch for easier review. The original DiscountedCfrTrainable class now remains completely untouched.

I introduced a ComboBox to choose between the level of half float usage in the GUI. The command line version has no interface yet, it's hardcoded disabled (so normal float version is used).

Not clear if even Turn+River option makes sense, it barely saves anything compared to River alone. So it could be simplified to a checkbox, which might cause less confusion to users.

The option does not gets saved yet with the saving of parameters.

I had a tough time getting the mainwindow.ui changed, the first few attempts caused crashes in the autogenerated ui_mainwindow.h. I finally got it running, but the layout is slightly changed.

I reverted the comment on #include "unistd.h", but I guess that should be solved generically. I use MSVC2015, and this does complain about not finding it. I didn't have the problem with your master tree, it got introduced in gui lately.

[bupticybee]

You are great! I will review and test before merge.

[bupticybee]

Can you also change the "Estimate Solving memory" to match your change? since now we have serval mode, it make sense to consider them when estimate memory space.

[bupticybee]

I reviewed other changes, compair results before and after, I think this change is good to merge as soon as the "Estimate Solving memory" thing is dealt with.

[maosatgithub]

I adopted the estimate by adding a factor of 0.5 for the half float version. As it is (and was before) only a rough estimate, I reduced the precision of display by a digit each.

As also the estimate is done based on the ui settings at the time of estimate, and not as the time of tree building, I added a warning to rebuild the tree after these settings are changed.

Before you merge, please consider my following suggestion:

We could use a third version, that combines the benefits of the r_plus_sum elimination with still using regular single floats for r_plus and cum_r_plus, thus the solving performance accuracy would be unchanged, speed decreases by about 10%. I would recommend to further use here half floats for the evs storage: that would not effect the solving itself, but would cause slightly less accurate EVS reporting (possibly effecting the 3rd digit, I guess).

I would change the selection box to chose between these total 3 variants with the option to save memory at cost of "nothing", "speed", or "speed and accuracy", and only apply this setting on the river, using original full floats otherwise.

What do you think?

[maosatgithub]

PS: Sorry for editing back and forth, I had some confusion when determining the speed cost of that 3rd option. Remembered 10%, the I measured 20% (but likely made a mistake), now its (less than) 10% again, also after removing some of the optimizations for half float, that actually cost something for the float version, as they added here not needed copy actions.

[bupticybee]

PS: Sorry for editing back and forth, I had some confusion when determining the speed cost of that 3rd option. Remembered 10%, the I measured 20% (but likely made a mistake), now its (less than) 10% again, also after removing some of the optimizations for half float, that actually cost something for the float version, as they added here not needed copy actions.

it's great news, your optimization really works well

[bupticybee]

I adopted the estimate by adding a factor of 0.5 for the half float version. As it is (and was before) only a rough estimate, I reduced the precision of display by a digit each.

As also the estimate is done based on the ui settings at the time of estimate, and not as the time of tree building, I added a warning to rebuild the tree after these settings are changed.

Before you merge, please consider my following suggestion:

We could use a third version, that combines the benefits of the r_plus_sum elimination with still using regular single floats for r_plus and cum_r_plus, thus the solving performance accuracy would be unchanged, speed decreases by about 10%. I would recommend to further use here half floats for the evs storage: that would not effect the solving itself, but would cause slightly less accurate EVS reporting (possibly effecting the 3rd digit, I guess).

I would change the selection box to chose between these total 3 variants with the option to save memory at cost of "nothing", "speed", or "speed and accuracy", and only apply this setting on the river, using original full floats otherwise.

What do you think?

I support the idea of adding the 3rd option, it's a great idea. The saved EV is only for showing to the user, it do not do anything in the compution process, I 100% support changing it to half float, it actually saved a lot of memory. In face EVs take up 1/3 of memory currently, I'd like to cut it in half.

[maosatgithub]

I pushed the changes for the 3rd option, it should be complete now and is ready to merge from my side.

[bupticybee]

I pushed the changes for the 3rd option, it should be complete now and is ready to merge from my side.

I will test and read the changes later today. Thanks for your contribution!

[bupticybee]

I do the tests, I think it works great, merging.

Thanks you for the contribution! This really make the solver a lot more useful.

[maosatgithub]

Thanks you for the contribution! This really make the solver a lot more useful.

As said before, the thanks is all on my side as well, and I am more than happy to be able to give something back for you sharing this great code base!