LibCity / Bigscity-LibCity

LibCity: An Open Library for Urban Spatial-temporal Data Mining
https://libcity.ai/
Apache License 2.0
937 stars 168 forks source link

HGCN: Adjacent Matrix may need to be converted to Laplacian Matrix #306

Closed SonghuaHu-UMD closed 2 years ago

SonghuaHu-UMD commented 2 years ago

Hello! One possible bug in the implementation of HGCN.

The graph convolution in the paper "Hierarchical Graph Convolution Networks for Traffic Forecasting", is directly borrowed from "GraphWaveNet for Deep Spatial-Temporal Graph Modeling".

From the two papers, they have three convolution operations using two diffusion matrices and one adaptive matrix (see Eq. (4) in the HGCN paper and Eq. (6) in the GWNET paper).

The implementation of HGCN in LibCity directly uses the Original Adjacent Matrix to do the graph convolution, which may not be correct:

https://github.com/LibCity/Bigscity-LibCity/blob/065a2adf70df8563aa9638d062eef2a2e0515abf/libcity/model/traffic_speed_prediction/HGCN.py#L199-L200

One more step may be needed to process the Original Adjacent Matrix to the Laplacian Matrix. See the code provided by the original authors (https://github.com/guokan987/HGCN/blob/7ab2d8376cf98e77b5dad336a67bfb259e49b22d/util.py#L177):

def load_adj(pkl_filename, adjtype):
    sensor_ids, sensor_id_to_ind, adj_mx = load_pickle(pkl_filename)
    if adjtype == "scalap":
        adj = [calculate_scaled_laplacian(adj_mx)]
    elif adjtype == "normlap":
        adj = [calculate_normalized_laplacian(adj_mx).astype(np.float32).todense()]
    elif adjtype == "symnadj":
        adj = [sym_adj(adj_mx)]
    elif adjtype == "transition":
        adj = [asym_adj(adj_mx)]
    elif adjtype == "doubletransition":
        adj = [asym_adj(adj_mx), asym_adj(np.transpose(adj_mx))]
    elif adjtype == "identity":
        adj = [np.diag(np.ones(adj_mx.shape[0])).astype(np.float32)]
    else:
        error = 0
        assert error, "adj type not defined"
    return sensor_ids, sensor_id_to_ind, adj

I also read your implementation of GWNET and suppose you could directly borrow the function from GWNET to HGCN: https://github.com/LibCity/Bigscity-LibCity/blob/065a2adf70df8563aa9638d062eef2a2e0515abf/libcity/model/traffic_speed_prediction/GWNET.py#L293-L307

Again, thank you for all your efforts in implementing these models!