The only reason to introduce

inline int operator/(T d1, T d2) { return int(d1) / int(d2); }

is to allow this:

(eval - beta)

instead of

int(eval - beta)

because remaining patch works flawless given that T / T already results in
an int, not by physic but by C++ rules. So I'd avoid adding the above
operator, but I'd definitely keep the remaining patch that is nice and
further makes depth independent from ONE_PLY value.

IMO if variables don't change they should in fact be const. In more complex functions this makes code easier to understand and the compiler can potentially perform additional optimizations. I realize these functions are too simple to make a difference here but it is a good habit/policy in general. Just my 2 cents.

@mcostalba: T/T does not return int currently. It returns T. The current operators are dangerous and we could add an "explicit" in front of int arguments.

When you do T/T, the second argument gets implicitly casted into an int, and you use T operator/(T,int).

Even better, try to.compile:

int bonus = (depth/ONE_PLY) * (depth/ONE_PLY)

you think you're multiplying two ints, but you're multiplying two depth. To add insult to injury, the compiler complains about an ambiguity in operator* resolution. Again, an explicit would result in the correct error message here.

@mstembera: the SF coding style is to not use const in an excessively pedantic way. arguments passed by value are never const. it makes even less sense to change the coding style for the.specific case of trivial one liner functions.

if you want to be 100% const-pedantic, it is a different discussion for a different pull request, that would impact thousands of lines of code.

Mmmm... Yes you are right. I have reconsidered my comment : patch is good
as is.

I am also against const pedantic. Const is more a code annotation than a
help to compiler (that does not need it). IOW it is used by programmer to
catch subtle bugs that instead to pass silently are caught by compiler. But
in these very simple inliners there is no need of this safeguard.

On Friday, November 21, 2014, lucasart notifications@github.com wrote:

@mcostalba https://github.com/mcostalba: T/T does not return int
currently. It returns T. The current operators are dangerous and we could
add an "explicit" in front of int arguments.

When you do T/T, the second argument gets implicitly casted into an int,
and you use T operator/(T,int).

Even better, try to.compile:

int bonus = (depth/ONE_PLY) * (depth/ONE_PLY)

you think you're multiplying two ints, but you're multiplying two depth.
To add insult to injury, the compiler complains about an ambiguity in
operator* resolution. Again, an explicit would result in the correct
error message here.

—
Reply to this email directly or view it on GitHub
#128 (comment)
.

Just pushed another patch, to show some practical use cases of T/T returning int instead of T. Also goes in the direction of making things less ONE_PLY dependant.

I have added some comments in the code, please check them.

For me the changes look fine. T/T = int is basic physics.

@lucasart, @mcostalba: Are you both happy with the latest version. Or do you think that further changes are necessary.

It is ok for me.

On Sun, Nov 23, 2014 at 1:06 PM, Joona Kiiski notifications@github.com
wrote:

For me the changes look fine. T/T = int is basic physics.

@lucasart https://github.com/lucasart, @mcostalba
https://github.com/mcostalba: Are you both happy with the latest
version. Or do you think that further changes are necessary.

—
Reply to this email directly or view it on GitHub
#128 (comment)
.

Looks good to me.

yes, i have nothing to add to this . we could push the pedal further and make the search entirely ONE_PLY independant. but it's a bit of work, and it's not obvious that the result would he more readable as a result. plus the timing isn't good for big patches (c++11 merge awating with big merge conflicts).

There are no big merge conflicts with c++11, I have updated it so it's now
functional equivalent with upstream with only minor differences.

On Monday, November 24, 2014, lucasart notifications@github.com wrote:

yes, i have nothing to add to this . we could push the pedal further and
make the search entirely ONE_PLY independant. but it's a bit of work, and
it's not obvious that the result would he more readable as a result. plus
the timing isn't good for big patches (c++11 merge awating with big merge
conflicts).

—
Reply to this email directly or view it on GitHub
#128 (comment)
.

OK. Scheduled for merging.

On the subject of operator*() ambuity (see comment above), I tried:

template <typename T> T operator*(T d, explicit int x)
template <typename T> T operator*(explicit int x, T d)

but the compiler refused it, saying that explicit is only for constructors.

Is there no C++ magic to resolve this problem?

Maybe you won't like this solution but can't you just cast to int in order to tell the compiler which operator you want it to use?
((int)depth * (int)depth) / (ONE_PLY * ONE_PLY)

looks like we should be using c++11 enum class, and define explicit ctor from int. this would resolve the problem of operator*, and make a whole bunch of dangerous code mixing typed enum and int illegal, preventing to write inadvertently inhomogenous formulas. this may seem like overengineering, but isnt overengineering the c++ way of life…

let's come back to that when c++11 branch is merged into master.

@mstembera: no, i dont like it. i want to solve the problem cleanly, not hack my way around it. but you're missing the bigger picture. it's not just about Depth*Depth. i want a type system that prevents implicit int conversion, in order to prevent writing inadvertently inhomogenous (in the sense of Physics) formulas.

I understand. That's why I said maybe you won't like it :)

What about defining a
Depth operator_(Depth,Depth)
operator? Would that resolve the ambiguity and still be considered clean?
inline T operator_(T d1, T d2) { return T(int(d1) * int(d2)); }

The problem with the physics analogy(which I like otherwise) is that when you multiply two values with units say meters the result is in meters squared. However Depth doesn't really have any actual units even though we consider it a type separate from int. If Depth is unit less then Depth * Depth is also unit less and still just of type Depth. So I think the "Depth operator*(Depth,Depth)" should be ok. What do u think?

I don't understand what's the point of adding an operator*(Depth,Depth).

Patch is good as is and IMO does not need anything else.

I was just trying to help Lucas brainstorm on how to compile Depth * Depth without the operator ambiguity because he asked. I didn't mean to imply that there is anything wrong with the patch as is.