austenlamacraft / audio-mps Goto Github PK

@mencia

• In the loss

               self.r_reg * tf.real(tf.reduce_sum(tf.conj(self.R)*self.R))

Why reduce_mean rather than reduce_sum? You are choosing the relative weight of the main loss and the regularization. Taking the view that the whole thing is is a log probability suggests to me that it should be reduce_mean. What do you think?

• Line 158. num_samples = tf.size(signal). It looks to me like tf.size gives the total number of elements, whereas you want the batch size signal.shape[0].

• Line 159 and 163. Are you sure this tile and reshape is right? Did you check it? I think tf.stack is what you want here, as that adds an index.

• Line 152. tf.log(self._expectation_rho(rho)). Why isn't this tf.log(tf.trace(rho))?

Smaller things:

1. Lines 25-39. Are all the ifs necessary? You supply the matrix as an initializer. If it's None, it does nothing.

2. [self.rank_rho_0, self.bond_d] is the same as 2*[self.rank_rho_0]

3. self.W, self.Wx, self.Wy are not used for anything, only to build self.rho_0. Put them in another method, say _init_rho_0.

4. Line 173. In exp = tf.trace(tf.einsum('ab,cbd->cad', x, rho)) the tf.trace is redundant. Do it all with einsum.

5. _inc_loss_rho doesn't use signal argument (PyCharm would tell you that!).