lunarlander | Pygame based lunar Lander

This error is not related to your model. forward function only returns the probability distribution but what you need is the action and corresponded probability (output of Policy.act).

Change your code from

Your problem is not in the fit call, but in the loop that you have in the replay() method. Try always substituting loops by numpy operations in these cases, that make the operations much more agile.

Replace your replay method by the following one and let me know if it works faster for you

You can't use Dense layers on 2D inputs (i.e. inputs that are 3D, with the batch dimension). It will only take the last dimension as its input. Dense layers work with 1D input tensors, provided as a 2D batch.

Either add a Flatten(shape=-1) layer to flatten your input (e.g. to a (batch_size, 169*3) tensor), or use a convolutional network.

Also your self.A layer appears to provide the q-value for each action, given a state. The advantage would be the action values - the value estimate. I think that's what you're calculating in call()

If the flatten doesn't fix your output shape, I'd double check the shapes of the arguments for Q, and the shape of the returned Q. print(Q.shape) will be your friend for debugging!

I tried your code(and other things surrounding this), and I'd like to tell you there's a way but I don't think there is.

I would instead recommend using a utility class that supports this static functionality you're looking for.For example, a LanderUtility class that has static methods in it might solve this in a reasonable way.

Mostly though, I don't think of using static methods in that way in Java. The real power in what is going on with this abstract class is that you can count on(somewhat) a certain type of behavior from a child. Notably, for this to matter, the child needs to be instantiated in the first place and you can use a normal, non-static method and have the child implement that instead.

You need to keep the 'synchronized' statements. If you don't (though note that android, which isn't really java, may not be adhering to the same memory model as actual java), then any thread is free to make a temporary clone for any field of any instance it wants, and synchronize any writes to the clone at some undefined later point in time with any other thread's clone.

To avoid the issues with these 'clones'*, you need to establish CBCA relationships ("comes before/comes after") - if the thread model ensures that line X in thread A definitely ran after line Y in thread B, then any field writes done by line Y will guaranteed be visible in line X.

In other words, with the synchronized statements, if the mRunLock lock in your run() method has to 'wait' for the setRunning method to finish running, you just established a CBCA relationship between the two, and it's crucial because that means the mRun write done by setRunning is now visible. If you didn't, it may be visible, it may not be, it depends on the chip in your phone and the phase of the moon.

Note that boolean writes are otherwise atomic. So it's not so much about any issues that would occur if you read whilst the field is being written (that is not a problem in itself if the field's type is decreed as being atomic, which all primitives other than double and long are), it's ensuring visibility of any changes.

In plain jane java you'd probably use an AtomicBoolean for this and avoid using any synchronized anything. Note also that nesting synchronized() on different locks (you lock on mSurfaceHolder, and then lock on mRunLock) can lead to deadlocks if any code does it 'in reverse' (locks on mRunLock first, then locks on mSurfaceHolder).

Are you running into any problems with this code, or just wondering 'is it correct'? If the latter: Yes, it is correct.

*) Whilst this clone thing sounds tedious and errorprone, the only alternative is that any field write by any thread is immediately visible by any other thread. That would slow everything waaaaay down; the VM has no idea which writes have the potential to be read soon by another thread, and if you know anything about modern CPU architecture, each core has its own cache that is orders of magnitude (100 to 1000 times!) faster than system memory. This alternative of 'all writes must always be visible everywhere' would pretty much mean that fields can never be in any caches ever. That'd be disastrous for performance. This memory model is therefore basically a necessary evil. There are languages that don't have it; they tend to be orders of magnitude slower than java.

There are two issues with your code:

1. delta_t should be 1.0/50.0 according to the lunar_lander source