chaoshangcs / GTS

Discrete Graph Structure Learning for Forecasting Multiple Time Series, ICLR 2021.
Apache License 2.0
172 stars 30 forks source link

No use of ground truth graph for METR-LA & PEMS-BAY #23

Closed KL4805 closed 1 year ago

KL4805 commented 2 years ago

Dear Chao,

Many thanks to your code! I find some questions that I would like to ask you.

  1. It seems that for PEMS-BAY and METR-LA, you are using a kNN graph as ground truth for regularization, rather than the ground truth graph. (Line 54, supervisor.py). This seems different from your paper. Any implications on the final result?
  2. It seems that in your implementation, you are using a constant temperature rather than annealing the temperature. What is the implication on the final result?
  3. It seems that during testing, you are only sampling one graph for evaluation (ref here) rather than the 10 you mentioned in the paper. Any implication on the final results?

Nonetheless, I appreciate your efforts as this is a simple but effective work. Looking forward to the reply.

chaoshangcs commented 2 years ago

Hi, thank you so much for your valuable questions. The following are the answers based on my understanding.

  1. As shown in section 3.3, we use the KNN graph as a regularization to encourage sparsity in our experiments. That means our model hasn't saw any explicit "true" or ground truth graph during the training. It is consistent with our defined problem that the explicit graph is unknown in most of cases.
  2. That's a great question. We disabled the function that anneals the temperature. If you have any findings, hope to get your valuable feedback.
  3. Thanks for pointing it out. Sampling more graphs will make the test results more stable. It is great to get these great feedbacks. Thanks!
KL4805 commented 2 years ago

In your paper (page 4), you said that "In practice, we anneal s progressively in training such that it tends to zero".

Also, in Appendix B, you said that "the expectation is computed as an average of 10 random samples".

Do they match what you say here?

chaoshangcs commented 2 years ago

Hi, sorry for the inaccurate response. Since I forgot some details about the code, so I rechecked our implementation today. Here is the new response for your question. First, we did explore the way to change the value "temp" as shown in row 304 of "supervisor.py". But I disabled it in the current version of code. You can enable it when you do the training. It don't have a significant influence to the performance. Second, we explored the "10 random samples" before. You can find that the code has a variable "self.num_sample" in row 26 of "supervisor.py". But it seems I forgot to add one row "for sample in range(self.num_sample):" in the evaluation function. Thanks for pointing it out. It might influence the performance a little bit.

KL4805 commented 2 years ago

Thanks for your explanation. I will try these.

Thanks anyway for your provided code implementation. They are very helpful.