Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question regarding possible error in calculating __pv_vector in NNAgent class. #111

Open
BruceDai003 opened this issue Nov 26, 2018 · 1 comment

Comments

@BruceDai003
Copy link

Thanks for your paper and project, I've been digging into the code for the past several days, and I felt that there might be an error in the implementation of calculating __pv_vector in the NNAgent class:

class NNAgent:
...
self.__pv_vector = tf.reduce_sum(self.__net.output * self.__future_price, reduction_indices=[1]) *\
                           (tf.concat([tf.ones(1), self.__pure_pc()], axis=0))

And here is the definition of __pure_pc funtion:

    # consumption vector (on each periods)
    def __pure_pc(self):
        c = self.__commission_ratio
        w_t = self.__future_omega[:self.__net.input_num-1]  # rebalanced
        w_t1 = self.__net.output[1:self.__net.input_num]
        mu = 1 - tf.reduce_sum(tf.abs(w_t1[:, 1:]-w_t[:, 1:]), axis=1)*c

        return mu

From my understanding, function __pure_pc() calculates the shrinkage factor due to re-balancing, this is a little bit different from the paper, where the mu was not subtracted from 1.
Let's get back to the calculation of self.__pv_vector, the term before * is the net value of the portfolio at the end of each time period(Let's call this term NV). You want to calculate the net value of the portfolio at the beginning of the next time period if I interpreted it correctly. But (tf.concat([tf.ones(1), self.__pure_pc()], axis=0)) doesn't fit in your purpose here.
Because NV[0] is the net value of the portfolio at the end of the first time period in this batch. It should multiply by the first shrinkage factor in self.__pure_pc() to get the net value of the portfolio at the beginning of the second time period relative to at the beginning of the first time period , instead of multiplied by 1 here.

Thus, my suggested correction would look like this:

self.__pv_vector = tf.concat([tf.ones(1), 
                                      tf.reduce_sum(self.__net.output * self.__future_price, axis=1)[:-1] *
                                      self.__pure_pc()], axis=0)
@cgebe
Copy link

cgebe commented Dec 1, 2018

I raised concerns regarding the computation of mu as well, see #99 (comment)

Especially, if you use a batch_size of 1, the error gets apparent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants