Declaring variables mid-function changes results

BlitzMax Forums/BlitzMax Programming/Declaring variables mid-function changes results

JoshK(Posted 2008) [#1]
If I declare the variables x,y,z, and w at the top of the function, the results are correct. If I leave them where they are, the results are incorrect rotations. If I comment out the KeyHit() command or add a Delay(1) command, the results are correct. Any printing or debugging I do makes the results correct, and masks the problem.

I wish I could give a better example, but any changes I make hide the problem. Any idea why this could be?

	Method Rotation:TQuat()
		Rem
		Local sc:TVec3=Scale()
		Local iv:TVec3=Vec3(ix,iy,iz).Scale(1/sc.x)
		Local jv:TVec3=Vec3(jx,jy,jz).Scale(1/sc.y)
		Local kv:TVec3=Vec3(kx,ky,kz).Scale(1/sc.z)
		
		Local t#=iv.x+jv.y+kv.z+1
		If t>0
			t=Sqr(t)*2
			x=(kv.y-jv.z)/t
			y=(iv.z-kv.x)/t
			z=(jv.x-iv.y)/t
			w=t/4
		Else If iv.x>jv.y And iv.x>kv.z
			t=Sqr(iv.x-jv.y-kv.z+1)*2.0
			x=t/4
			y=(jv.x+iv.y)/t
			z=(iv.z+kv.x)/t
			w=(kv.y-jv.z)/t
		Else If jv.y>kv.z
			t=Sqr(jv.y-kv.z-iv.x+1)*2.0
			x=(jv.x+iv.y)/t
			y=t/4
			z=(kv.y+jv.z)/t
			w=(iv.z-kv.x)/t
		Else
			t=Sqr(kv.z-jv.y-iv.x+1)*2.0
			x=(iv.z+kv.x)/t
			y=(kv.y+jv.z)/t
			z=t/4
			w=(jv.x-iv.y)/t
		EndIf
		Return Quat( x,y,z,w )
		EndRem
		Local fourWSquaredMinus1:Float = +ix+jy+kz
		Local fourXSquaredMinus1:Float = +ix-jy-kz
		Local fourYSquaredMinus1:Float = +jy-ix-kz
		Local fourZSquaredMinus1:Float = +kz-ix-jy
		Local biggestIndex=0
		Local fourBiggestSquaredMinus1:Float=fourWSquaredMinus1
		If fourXSquaredMinus1 > fourBiggestSquaredMinus1
			fourBiggestSquaredMinus1 = fourXSquaredMinus1 
			biggestIndex=1
		EndIf
		If fourYSquaredMinus1 > fourBiggestSquaredMinus1
			fourBiggestSquaredMinus1 = fourYSquaredMinus1 
			biggestIndex=2
		EndIf
		If fourZSquaredMinus1 > fourBiggestSquaredMinus1
			fourBiggestSquaredMinus1 = fourZSquaredMinus1 
			biggestIndex=3
		EndIf
		Local biggestValue:Float=Sqr(fourBiggestSquaredMinus1+1.0)*0.5
		Local mult:Float=0.25/biggestValue
		Local x#,y#,z#,w#
		Select BiggestIndex
			Case 0
				w#=BiggestValue
				x#=(jz-ky)*mult
				y#=-(kx-iz)*mult
				z#=-(iy-jx)*mult
			Case 1
				x#=-BiggestValue
				w#=-( jz - ky )*mult
				y#=( iy + jx )*mult
				z#=( kx + iz )*mult
			Case 2
				y#=BiggestValue
				w#=-(kx-iz)*mult
				x#=-(iy+jx)*mult
				z#=(jz+ky)*mult
			Case 3
				z#=-BiggestValue
				w#=(iy-jx)*mult
				x#=(kx+iz)*mult
				y#=-(jz+ky)*mult
		EndSelect
		'If KeyHit(KEY_L) Notify BiggestIndex
		Return quat(-x,y,z,w)

		
		
		Rem
		'testing code:
		Global mode=0
		If KeyHit(KEY_N)
			mode:+1
			'Notify mode
		EndIf
		If KeyHit(KEY_L) Notify mode
		Select mode		
			Case 0 Return quat(x,y,z,w)
			Case 1 Return quat(x,y,z,-w)
			Case 2 Return quat(x,-y,z,w)
			Case 3 Return quat(x,y,-z,w)
			Case 4 Return quat(-x,-y,z,w)
			Case 5 Return quat(x,-y,-z,w)
			Case 6 Return quat(-x,y,z,w)
		EndSelect
		EndRem
		
	EndMethod



ImaginaryHuman(Posted 2008) [#2]
Declare them as local at the top anyway.


JoshK(Posted 2008) [#3]
Why.


sswift(Posted 2008) [#4]
Because it's standard to do that, and it makes the code easier to read.

And obviously the answer to your problem is there's a bug in the compiler. You'll just have to work around it.


Dreamora(Posted 2008) [#5]
That code was definitely not tested in Strict / SuperStrict, otherwise the case "where it is defined where it is now" would not compile at all as you use x,y,z,w already in the first ifs.
by having the local below the first usage, it will just reset the content or result in other undefined behaviors as local is not meant for non strict as it does not do anything anyway, the only local scope is a function/method.

Without Strict / SuperStrict, forget local.
It will not work.

Scopes work seriously different without Strict / SuperStrict (and the language is considerably slower in many cases as the GC can not clean most of the stuff within a usefull amount of time)


sswift(Posted 2008) [#6]
No he doesn't. He just didn't bother to delete a bunch of code he commented out.



Scopes work seriously different without Strict / SuperStrict (and the language is considerably slower in many cases as the GC can not clean most of the stuff within a usefull amount of time)



Where do you get this information? I've never seen anyone mention any such thing.


ziggy(Posted 2008) [#7]
Well, that's true.
Code bloks on strict and superstrict have their own local definition scope. That makes code faster in some situations and much more easy to mantain

Example:
SuperStrict
test() 
Function test() 
	Local x:Int = 5
	If True
		Local x:Int = 6
		Print "INSIDE IF:" + x
	End If
	Print "OUTSIDE IF:" + x
End Function


that's one of the strong points of using strict and superstrict.

removing the superstrict directive from the avobe sample will make the code buggy and won't compile. Those are scope-based local variables.
If you ever use BLIde, the code analizer can help you optimizing the usage of local variables when working on strict or superstrict modes.

This code produces the output:
INSIDE IF:6
OUTSIDE IF:5


Obviously local scopes on code blocks overlap properly in a nested way, so locals defined outside a nested block is still accesible from inside the nested block, unless there's another local with the same name. In this example, the variable X defined inside the IF block is hiding a external variable called X. So you can define you own locals on nested blocks without worring if you're modifying properly another local variable in the container block.


Dreamora(Posted 2008) [#8]
Don't see how he could have removed anything that would not mean his code is broken for Strict/SuperStrict. If x is not defined before the first if chain starts where it is used.
-> can not be strict so its no BM code but Blitz3D code as nonstrict is only there for exactly this use, the BB importer (its the only mode that allows goto and implicit object -> int assignements)

Thats a known fact since strict was introduced:

Without Strict, you are in Blitz3D / Plus compatibility mode.
This mode knows only 1 type of scope: Function/Method. Everything else is within the same scope, so locals are wothless as only the ending of a function/method will remove "locals". Using them within IF will lead to dublicate identifier error etc

And the speed drop is caused by exactly that, as stuff remains much longer (if not forever) within scope and is not cleared unless you forcefully null them. Something that has a large impact if you normally use local within ifs, loops etc.

Not programming with at least Strict(better SuperStrict to prevent errors through implicit int casts) is considered slugish programming and is not acceptable for BM, as it disables most of BMs optimations for the math end.


sswift(Posted 2008) [#9]

Don't see how he could have removed anything that would not mean his code is broken for Strict/SuperStrict. If x is not defined before the first if chain starts where it is used.



Did you not read what I wrote? That code is COMMENTED OUT.

Here is his code with all the commented out stuff removed:

Method Rotation:TQuat()

	Local fourWSquaredMinus1:Float = +ix+jy+kz
	Local fourXSquaredMinus1:Float = +ix-jy-kz
	Local fourYSquaredMinus1:Float = +jy-ix-kz
	Local fourZSquaredMinus1:Float = +kz-ix-jy
	Local biggestIndex=0
	Local fourBiggestSquaredMinus1:Float=fourWSquaredMinus1

	If fourXSquaredMinus1 > fourBiggestSquaredMinus1
		fourBiggestSquaredMinus1 = fourXSquaredMinus1 
		biggestIndex=1
	EndIf

	If fourYSquaredMinus1 > fourBiggestSquaredMinus1
		fourBiggestSquaredMinus1 = fourYSquaredMinus1 
		biggestIndex=2
	EndIf

	If fourZSquaredMinus1 > fourBiggestSquaredMinus1
		fourBiggestSquaredMinus1 = fourZSquaredMinus1 
		biggestIndex=3
	EndIf

	Local biggestValue:Float=Sqr(fourBiggestSquaredMinus1+1.0)*0.5
	Local mult:Float=0.25/biggestValue
	Local x#,y#,z#,w#

	Select BiggestIndex

		Case 0
			w#=BiggestValue
			x#=(jz-ky)*mult
			y#=-(kx-iz)*mult
			z#=-(iy-jx)*mult

		Case 1
			x#=-BiggestValue
			w#=-( jz - ky )*mult
			y#=( iy + jx )*mult
			z#=( kx + iz )*mult

		Case 2
			y#=BiggestValue
			w#=-(kx-iz)*mult
			x#=-(iy+jx)*mult
			z#=(jz+ky)*mult

		Case 3
			z#=-BiggestValue
			w#=(iy-jx)*mult
			x#=(kx+iz)*mult
			y#=-(jz+ky)*mult
	EndSelect

	Return quat(-x,y,z,w)

EndMethod


You show me where he's using x,y,z, or w before he declares them.


sswift(Posted 2008) [#10]

Code bloks on strict and superstrict have their own local definition scope. That makes code faster in some situations and much more easy to mantain.



I had no idea you could even do that. If I saw that code in someone else's function, I would have assumed they were messy coders, declaring variables in the middle of code instead of at the top, and on top of that declaring the same variable twice!

Actually, even knowing this now I would still consider it a bad practice to declare X twice in the same function even if it is in a different scope. And I'm not sure that even though I now know this I would use it as it makes the code look messy.

But does it really optimize things? That's counterintutive. That seems like delcaring and freeing a bank over and over in a loop. Maybe it speeds up ints and stuff which can be stored in registers though? But what if those are arrays you're declaring and undelcaring there? That can't be good...


ziggy(Posted 2008) [#11]
Yes it optimize things. Obviously it can make the code messy if you use variable names like X. but how often have you need a aux variable inside a IF block (swap values as instance), and why should this variable be alive in the whole life of the function? coding properly, that is, using clear variable names, can make this practicle a very good practicle, specially becouse you can have an idea on what's the variable used for depending on where it is created. IMHO that makes things clearer than just placing all of them at the begining of a function ala Pascal.


Dreamora(Posted 2008) [#12]
you can not declare x twice unless its in different scopes.
So if you define x at the beginning of the function it will remain defined till its end.

And yes it optimizes things as each local will be held in cache, so if the local can be removed this means less data to keep in sync with the RAM and registers.

And yes you would not use local within a loop, that actually would be counterproductive.

But if you have if <loop> endif, you would for example declare the local within the if.
That ensures that the variable does not get generated when the if is never met -> no allocation for something not used at all, unlike it would happen otherwise.

Arrays, as all objects, are references -> ints as well.
And for numerics local is a whole different thing as they never get GC cleaned, they are no objects at all.

The whole thing is quite complex and powerfull but you can struggle quite easily as well.
Don't ask me on what strict has an influence at whole, I know for sure that it is scopes and therefor the whole GC process and its known that not using it can speed up the whole performance by 2-5 times if used correctly to enforce register / level 1 cache usage where usefull.


sswift(Posted 2008) [#13]
Ziggy:
I don't exaclty disagree wirh you. I have for a while been toying with the idea of declaring variables closer to where they're used simply because it's a pain to have to go searching for them when the place they're being modified is somewhere else. Like a function which might return some data in some globals. Declaring those near the function that uses them would keep the stuff together. On the other hand it would make it harder to look at the set of functions and see what globals are there for you to access. So... tradeoffs.

Of course this isn't exactly what you're talking about doing.

So maybe I'll take another look at declaring variables in different places. But I'm not gonna do this crap very often.

Local biggestValue:Float=Sqr(fourBiggestSquaredMinus1+1.0)*0.5

I hate when people do that. I only do that when I have to because I need to set a variable at the start of a type delcaration. It looks messy.


sswift(Posted 2008) [#14]
Ps:
I was practically forged in Turbo Pascal. :-) And that is why I use Blitz. I hate C because everything is all hard to read symbols. I like being able to go back to my code after six months and not wonder what If *xyz->blah || 0xFF == blah >> 2 { x++ } means. :-)


Czar Flavius(Posted 2008) [#15]
Like a function which might return some data in some globals.
Globals??????? Regular useage????? AHHHHHHHH!!!!!!!!!!! There's a return keyword for a reason ;)

Because it's standard to do that, and it makes the code easier to read.
It's standard practice to declare variables as close as possible to the scope in which they are first needed. Declaring all required variables at the start of a function might seem neat or organised for a small function, but for a non-trivial function it makes code harder to read and maintain.


sswift(Posted 2008) [#16]

Globals??????? Regular useage????? AHHHHHHHH!!!!!!!!!!! There's a return keyword for a reason ;)



Well, I don't use globals for functions much anymore, but the sort of case I'm talking about would be something like, say, a math function which transforms a point, and needs to return the new one.

IMO, creating a type and requiring someone to create an instance of said type just to get a point back from a simple math function is a crappy way of doing things.

And don't even think about suggesting the use of Var. I hate that because when you're looking at the function call you can't tell if the value is being passed into the function or being set.

So the way I handle that sort of thing now is instead of having globals, I'll try to wrap the functions into a type. And to avoid having people accessing variables of that type like they would a global and instead have them access functions which they get in a nice list on the right side of the screen, I'll have the fields in the type which I would formerly have made globals, with underscores, and then have functions for accessing those fields with the names without underscores.

Of course that still seems really stupid to me, having to make functions just so people can go X = Sprite.TFormedX#() instead of X = Sprite.TFormedX#, but I dunno I guess I'm doing it that way because I see BlitzMax working that way and I want to be consistent with that and I'm assuming Mark knows something I don't about proper code design, so later maybe this won't come back to bite me.

The only real advantage I see to doing it with wrapped functions like that is that it's always consistent how you access the data, and something like Paused() can be changed later so it maybe doesn't return the value of _Paused% but rather _Paused%|_Minimized%, and your old code won't break when you do that. Probably.


It's standard practice to declare variables as close as possible to the scope in which they are first needed.


Well maybe in C++, I don't know. But I'm old school baby, and I never learned to do things that way. :-)


Gabriel(Posted 2008) [#17]
And don't even think about suggesting the use of Var. I hate that because when you're looking at the function call you can't tell if the value is being passed into the function or being set.

Bear in mind that often times with Var, you're not passing values in *or* setting them, you're actually doing both, but I can see how someone might not find it readable.

Personally I like Var and I use it extensively, but if you don't, you could always declare the parameters as int pointers or float pointers and then you will be able to tell if the value is being passed in or being set because you'll have to use VarPtr() in the function call if you want to set it. I completely agre with your comments about creating an object just to return something. I consider that very bad practice. I try to only ever create a type if it's going to be used for more than one cycle.


JoshK(Posted 2008) [#18]
Dreamora, there is a big REM block at the top of the code where I commented out Mark's old code.

I think this has unveiled a BlitzMax bug. Unfortunately the nature of it makes it extremely difficult to debug.


marksibly(Posted 2008) [#19]
Hi,

Try it with a dummy KeyHit(), eg:

Function KeyHit(n)
End Function

...just to see if it's a weird KeyHit side effect or not (I suspect not).


ziggy(Posted 2008) [#20]
Mark could confirm it, but usually compilers tend to use registers for local scope declared variables so they are faster on release mode. That's also another additional reason to use scope-related locals on strict or superstrict.


TomToad(Posted 2008) [#21]
Have you tried it on another computer? It appears to me that x,y,z,w are being optimized by doing the math in CPU registers. When declaring them at the top instead of right before they are used, you are causing blitzMax to store the variables in memory instead. Also Print, DebugLog, etc... would also necessitate putting the values into memory.
I remember some old pentiums having bugs in the math coprocessor, possibly the same thing happening here?


JoshK(Posted 2008) [#22]
Calling a dummy function makes the error not happen.

I wish I could provide a working example other than a description of "screwed up rotation". This is very strange.

I am running in Strict mode, by the way.

	Method Rotation:TQuat()		
		Local mult:Float
		Local fourWSquaredMinus1:Float = +ix+jy+kz
		Local fourXSquaredMinus1:Float = +ix-jy-kz
		Local fourYSquaredMinus1:Float = +jy-ix-kz
		Local fourZSquaredMinus1:Float = +kz-ix-jy
		Local biggestIndex=0
		Local fourBiggestSquaredMinus1:Float=fourWSquaredMinus1
		If fourXSquaredMinus1 > fourBiggestSquaredMinus1
			fourBiggestSquaredMinus1 = fourXSquaredMinus1 
			biggestIndex=1
		EndIf
		If fourYSquaredMinus1 > fourBiggestSquaredMinus1
			fourBiggestSquaredMinus1 = fourYSquaredMinus1 
			biggestIndex=2
		EndIf
		If fourZSquaredMinus1 > fourBiggestSquaredMinus1
			fourBiggestSquaredMinus1 = fourZSquaredMinus1 
			biggestIndex=3
		EndIf
		Local biggestValue:Float=Sqr(fourBiggestSquaredMinus1+1.0)*0.5
		mult=0.25/biggestValue
		Local x#,y#,z#,w#
		Select BiggestIndex
			Case 0
				w#=BiggestValue
				x#=(jz-ky)*mult
				y#=-(kx-iz)*mult
				z#=-(iy-jx)*mult
			Case 1
				x#=-BiggestValue
				w#=-( jz - ky )*mult
				y#=( iy + jx )*mult
				z#=( kx + iz )*mult
			Case 2
				y#=BiggestValue
				w#=-(kx-iz)*mult
				x#=-(iy+jx)*mult
				z#=(jz+ky)*mult
			Case 3
				z#=-BiggestValue
				w#=(iy-jx)*mult
				x#=(kx+iz)*mult
				y#=-(jz+ky)*mult
		EndSelect
		KeyHit(KEY_N)
		Return quat(-x,y,z,w)
	EndMethod
	
	Function KeyHit(blah)
	EndFunction



Dreamora(Posted 2008) [#23]
This makes no sense that this should make a difference, otherwise all games would be wrong released so far.

Is this within a module or a file thats imported, not included (on Strict/SuperStrict)? If so, is the module beeing imported with the input handling?


JoshK(Posted 2008) [#24]
This is in the main program.

I know this makes no sense at all. That is why I am posting this here.


Dreamora(Posted 2008) [#25]
Well what is the environment within which this code is?
A module? included in a main program?
What modules are imported, what is the framework, strict/superstrict?
Which BM are you using? (I assume 1.28 SVN as you surely want to be on the current version of the BCC not the one with the reflection bug++)


JoshK(Posted 2008) [#26]
It's an include in the main prog. I thought I made that clear.


Dreamora(Posted 2008) [#27]
Still more points remaining:
What modules are imported, what is the framework, strict/superstrict?
Which BM are you using? (I assume 1.28 SVN as you surely want to be on the current version of the BCC not the one with the reflection bug++)



marksibly(Posted 2008) [#28]
Can you put together a single file demo of the problem?


JoshK(Posted 2008) [#29]
I hope so.


JoshK(Posted 2008) [#30]
Yes, here is a program demonstrating the bug. Most of the time it comes out the same, but case 8, 14, and 18 give different results:


Here is my output log:
Building bug
Compiling:bug.bmx
flat assembler  version 1.66
3 passes, 24505 bytes.
Linking:bug.exe
Executing:bug.exe
1
Quat(0.505490303,-0.533333719,-0.307793617,0.604398549)
Quat(0.505490303,-0.533333719,-0.307793617,0.604398549)

2
Quat(0.479863316,0.697153628,-0.357858539,-0.394519120)
Quat(0.479863316,0.697153628,-0.357858539,-0.394519120)

3
Quat(-0.381797940,-0.183105320,-0.623658419,0.657079101)
Quat(-0.381797940,-0.183105320,-0.623658419,0.657079101)

4
Quat(0.260478765,0.870145679,0.391556114,0.147245243)
Quat(0.260478765,0.870145679,0.391556114,0.147245243)

5
Quat(0.0375171192,0.707903206,0.382147193,-0.592814445)
Quat(0.0375171192,0.707903206,0.382147193,-0.592814445)

6
Quat(0.789529085,0.0312671997,0.198185980,0.579990089)
Quat(0.789529085,0.0312671997,0.198185980,0.579990089)

7
Quat(0.742616892,-0.284792781,-0.244056195,0.554842114)
Quat(0.742616892,-0.284792781,-0.244056195,0.554842114)

8
Quat(-0.194731668,-0.612208784,-0.730031371,-0.233096883)
Quat(0.730031371,-0.233096883,0.194731668,-0.612208784)

9
Quat(-0.529806376,0.429777414,-0.405184805,0.608622909)
Quat(-0.529806376,0.429777414,-0.405184805,0.608622909)

10
Quat(0.557507277,0.593058288,0.458249658,0.357036322)
Quat(0.557507277,0.593058288,0.458249658,0.357036322)

11
Quat(0.743423522,0.224995196,0.133408666,0.615549266)
Quat(0.743423522,0.224995196,0.133408666,0.615549266)

12
Quat(0.638532400,0.211647302,-0.413198650,0.613798618)
Quat(0.638532400,0.211647302,-0.413198650,0.613798618)

13
Quat(0.661161602,0.473749906,0.578552246,-0.0608576164)
Quat(0.661161602,0.473749906,0.578552246,-0.0608576164)

14
Quat(0.432375759,-0.635354221,-0.639105976,0.0303268787)
Quat(0.639105976,0.0303268787,-0.432375759,-0.635354221)

15
Quat(0.633128643,-0.0820736513,0.297141016,0.710013568)
Quat(0.633128643,-0.0820736513,0.297141016,0.710013568)

16
Quat(0.385254771,0.691897929,0.574016333,0.208233491)
Quat(0.385254771,0.691897929,0.574016333,0.208233491)

17
Quat(0.616475344,-0.275921464,0.142099261,0.723625183)
Quat(0.616475344,-0.275921464,0.142099261,0.723625183)

18
Quat(-0.100223772,0.0979351997,-0.818663001,-0.556915581)
Quat(0.818663001,-0.556915581,0.100223772,0.0979351997)

19
Quat(-0.582106888,0.392956883,-0.268982202,0.659078956)
Quat(-0.582106888,0.392956883,-0.268982202,0.659078956)

20
Quat(-0.206659749,0.683476806,0.674812436,-0.186492458)
Quat(-0.206659749,0.683476806,0.674812436,-0.186492458)


Process complete



Dreamora(Posted 2008) [#31]
Interesting result: with debug it does not bug!
What I found out as well is that only 8,14,18 end in case 3 and that are actually those 3 that are always wrong so I would guess that there is a strange relation between them


Zeke(Posted 2008) [#32]
debug output



ziggy(Posted 2008) [#33]
I think this could be caused by the usage of Registers to store locals on release mode, as rounding works in a different way.


Floyd(Posted 2008) [#34]
I think this could be caused by the usage of Registers to store locals on release mode, as rounding works in a different way.

That was my first thought.

But storing registers to memory causes them to lose precision. This code gives better values with debug on. That's the opposite of what I expected.


Floyd(Posted 2008) [#35]
Here's an alternate version of the example.



And the output is
     Original angles: Vec3(-60.0000000,30.0000000,-120.000000)
        Becomes quat: Quat(0.0473671779,0.530330062,0.789149106,0.306186199)
    Mat to quat good: Quat(-0.0473671854,-0.530330062,-0.789149106,-0.306186199)
     Mat to quat bad: Quat(0.789149106,-0.306186199,0.0473671854,-0.530330062)
Restored angles good: Vec3(-59.9999924,29.9999924,-119.999992)
 Restored angles bad: Vec3(59.9999924,150.000000,-119.999992)
 BRL restored angles: Vec3(-59.9999733,29.9999962,-119.999977)