Add Math.square to compute the square of a number without the need to repeat…
Description
Details
Event TimelineComment Actions When did i accept this? i had comments (not important ones i agree) which were not taken into account and still does not think what has been implemented is the cleaner way to do it. Comment Actions " Math.square accepted by mimo" Where did I state that you accepted the patch? (Also I did take your comments into account in the form of stating why I disagree with them. Notice that originally you disagreed with Math.square as well which is why I made that differential revision proposal to begin with) Comment Actions I also never accepted Math.square, i only said in a pm that i was fine with it. For me, it only means i don't object or i don't care, but not that i support it as the commit message seems to imply. In fact here it was "i don't care", and certainly not something relevant to be put in a commit message of a patch i didn't look with attention. When you commit without formal acceptation in phabricator, you must assume your acts and not hide yourself by involving other people.
Once again, you distord what i said: i never objected to Math.square, i only said that it couldn't be faster than the direct multiplication. And as it happens that the difference is negligible, i don't care. Then coming to the concern, i would have expected that you did some profiling before commiting it, which does not seem to be the case: i've tried this simple test inside the ai Index: binaries/data/mods/public/simulation/ai/petra/_petrabot.js =================================================================== --- binaries/data/mods/public/simulation/ai/petra/_petrabot.js (révision 20330) +++ binaries/data/mods/public/simulation/ai/petra/_petrabot.js (copie de travail) @@ -89,6 +89,25 @@ m.PetraBot.prototype.OnUpdate = function(sharedScript) { + let x = [0, 0]; + let y = [0, 0]; + let time0 = Date.now(); + for (let i = 0; i < 50000; ++i) + { + x[0] = i; + y[1] = i; + let dist = API3.SquareVectorDistance(x, y); + } + warn(" new time " + (Date.now() - time0)); + time0 = Date.now(); + for (let i = 0; i < 50000; ++i) + { + x[0] = i; + y[1] = i; + let dist = Math.square(x[1] - x[0]) + Math.square(y[1] - y[0]); + } + warn(" old time " + (Date.now() - time0)); + if (this.gameFinished) return; and the result is WARNING: new time 130
WARNING: old time 5
WARNING: new time 135
WARNING: old time 0
WARNING: new time 133
WARNING: old time 0
WARNING: new time 128
WARNING: old time 1
nice for a patch supposed to improve performances. I've never counted the number of distance calls by the ai every turns, but i think it could reach several ten thousands on turns with building placement (and maybe even more on large maps). In fact, i suppose this factory lag you've added is because of the spread operator ... that you've put in the ai function. Then on a less important side, why calling the function euclidDistance and not distance? that's just pedantic, gives the impression for anybody not used to maths that it does something complicated and doubles the number of characters needed with no reason. Comment Actions That sounds exactly equivalent to me.
"acceptance is person's assent to the reality of a situation, recognizing a process without attempting to change it or protest it." according to wikipedia.
The sole reason why I have stated that you accepted Math.square in the commit message and created a proposal to begin with was to establish whether
The commit message exactly states the results of the discussions of the patch.
I (apparently falsely) suspected that https://code.wildfiregames.com/rP20263#23668 meant disagreement of adding the functions, which is why I asked again before adding them. The test compares the changed function with an inlined call, but it should compare the prior function with the new function. After removing the spread operator, the before/after times still differ about 15%.
This euclid distance diff was a duplication removal patch.
There are many distance measures https://en.wikipedia.org/wiki/Category:Similarity_and_distance_measures Comment Actions Unfortunately, you are still lying. I put back my own words "i don't see any drawbacks to have it" in answer to the pm you sneakily send me to ask my advice, never mentionning that this will be taken as a formal acceptation for a commit. From somebody who has some ethics, i would have expected at worse some regrets, and at best excuses, for having used improperly my name. It seems that's something completely alien to you. That's a behaviour i would never accept in real live, so i won't accept it here.
completely irrelevant and stupid argument (that cannot explain factor 100 difference) to once again try to hide the problem you created.
Ah at last!
That's the exact point. How many people knows what a euclidian distance is? while everybody knows what a distance is. And once again, the argument of lot of distances in wikipedia is just stupid. It is only when we use a term in a meaning different from the usual one that we have to describe it in the name. Comment Actions According to my understanding of language, "Math.square accepted by someone" exactly means that that the person stated being fine with the introduction of a Math.square function, i.e. this part of the diff: /**
* Get the square of a number without repeating the value and without calling the slower Math.pow.
*/
Math.square = function(x)
{
return x * x;
};I won't participate in taking this dispute a personal level. Comment Actions Nonsense, "accepted" put in a commit message means much more than "i don't see any drawback".
It is me who didn't ask anything, but you sent me the pm without giving the exact context of why you asked me that question and then quoted wrongly my answer on something i didn't want to be involved with. So i'm still waiting for some excuses. Comment Actions " I'm also ok with EuclidDistance functions." What does "Math.square accepted by mimo" mean to you? Comment Actions That is becoming completely crazy, and i start to believe you are completely stupid (sorry for the personnal aspect). You asked me "Would you be ok with me implementing these functions in D969 ?". I don't see how i could answer anything except yes to such a generic question. And i answered that in a very loose way (I don't see any drawback for the first, and also ok for the second). "Accepted" in a commit message means i support the way it is implemented (which i never said and i really hate when people make me say things i don't think). These are two different things, and i really find sneaky the way you behaved and still are behaving. Comment Actions That is exactly what happens here, the point seem rather clear: both of you value the same words differently. In which neither interpretation is wrong/lesser/better or whatever than the other. Mimo is right when he says he didn't meant to say the patch was ready for commit and nor he clearly set it was. Elexis interpreted that it was, which is a valueable interpretation of "having no drawback". However you are making a flame-ware from it and that won't help the situation. So can both agree on trying to be 1) more clear in what you say 2) less over-interpretative? (How hard both things even can be.) Comment Actions You stated in the PM that you are ok with the function definition:
When I stated Math.square accepted by mimo I merely meant the function definition not the calls to it. I regret the ambiguity of that sentence which I did not foresee. Comment Actions Can we just stop this discussion? Comment Actions Spread operator slowness fixed by rP20351. |