steveKapturowski / tensorflow-rl

Implementations of deep RL papers and random experimentation
Apache License 2.0
177 stars 47 forks source link

about fast_cts.pyx #13

Closed lezhang-thu closed 7 years ago

lezhang-thu commented 7 years ago

In fast_cts.pyx, there is the following code:

        for i in range(self.height):
            for j in range(self.width):
                context[0] = obs[i, j-1] if j > 0 else 0
                context[1] = obs[i-1, j] if i > 0 else 0
                context[2] = obs[i-1, j-1] if i > 0 and j > 0 else 0
                context[3] = obs[i-1, j+1] if i > 0 and j < self.width-1 else 0

which confuses me, as it's inconsistent with the code from cts_density_model.py, where the same logic appears as:

        for i in range(self.factors.shape[0]):
            for j in range(self.factors.shape[1]):
                context[3] = obs[i, j-1] if j > 0 else 0
                context[2] = obs[i-1, j] if i > 0 else 0
                context[1] = obs[i-1, j-1] if i > 0 and j > 0 else 0
                context[0] = obs[i-1, j+1] if i > 0 and j < self.factors.shape[1]-1 else 0

Please note the order has been different: context[3 ... 0] and context[0 ... 3]. Thanks!

steveKapturowski commented 7 years ago

Good catch! The [3 ... 0] ordering should be the correct one since the most significant symbol is popped from the end of the list. It's fixed in this commit: bcc9b2a966d7e5dee53f2a1fef1d5c355af85a79