New feature: Integration of ogbl-vessel specific code #31
Conversation
|
Hi @jqmcginnis! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
facebook-github-bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
@jqmcginnis Hi Julian, thank you very much for the extending SEAL to support ogbl-vessel! We tested the changes and everything looks fine. We also ran it on ogbl-vessel with (1hop/5epochs/5%/use_feature/use_weight) hyperparameters and got 80.78 highest validation AUC and 80.75 final test AUC. Better results are possible with tuning. Cheers, |
|
@muhanzhang Thank you very much for the integration! 🙂 Regarding submission of the results, did you do grid search for the other algorithms / previous submissions? Cheers, |
|
Hi @jqmcginnis , we did not do grid search for this result, and mostly only did minimal hyperparameter tuning for previous SEAL submissions. I saw that in your paper you have much higher results (90% AUC) for SEAL. Since you have better understanding of the dataset and perhaps have performed grid search for SEAL, I will appreciate it if you can submit your SEAL results to the leaderboard. Also, please feel free to refer to this repository in your ogbl-vessel official repository, and add the command to reproduce the SEAL results into the README file of this repo (or/and that of your repo). Thanks, |
|
I reported the results yesterday evening 🙂 - however they are not in the 90% ROC-AUC range, as the official ogbl-vessel graph differs from the VesselGraph paper in terms of node features. I will dedicate the upcoming days to clarify this in the VesselGraph Repo and we will probably update the archiv paper as well, in order to include average and std-dev for all results. Also, I will add ogbl-vessel to this repository's README and refer users of VesselGraph to this repo! Julian |
| if 'edge_neg' in split_edge['train']: | ||
| # use presampled negative training edges for ogbl-vessel | ||
| neg_edge = split_edge[split]['edge_neg'].t() | ||
|
|
||
| else: | ||
| new_edge_index, _ = add_self_loops(edge_index) | ||
| neg_edge = negative_sampling( | ||
| new_edge_index, num_nodes=num_nodes, | ||
| num_neg_samples=pos_edge.size(1)) | ||
| else: | ||
| neg_edge = split_edge[split]['edge_neg'].t() | ||
|
|
There was a problem hiding this comment.
Hi @jqmcginnis and @muhanzhang ,
I was looking at this change made for the addition of ogbl-vessel. If I am not wrong, won't this slightly change the logic for existing datasets?
For ex., previously, if we were running the training code for Cora, it would enter the if split == 'train' block instead and create negative edges. Is there any reason we were re-sampling for negative edges previously even though this information was available already in split_edge[split]['edge_neg'] created as part of get_pos_neg_edges()?
I apologize if my understanding is incorrect.
There was a problem hiding this comment.
Hi @venomouscyanide, thank you very much for sharing this!
I don't know why exactly this is/was the case. My guess would be that SEAL_OGB was built around the ogbl (link) datasets that (prior to ogbl-vessel) did not contain negative training edges, and thus sampling was mandatory, and consequently introduced in the code.
However, I would love to hear @muhanzhang's ideas with respect to this, and I am happy to address any issues, if the changes for ogbl-vessel cause any unintended behavior for other datasets (also non ogb ones!) 🙂
Hi SEAL team,
we are currently moving towards the official release of ogbl-vessel and would be very happy to have SEAL on the official leaderboard. Thus, I implemented some changes to the current code base to support the new graph. Please consider reviewing the changes and pushing the code to the main branch, if you are interested in supporting ogbl-vessel as well 🙂. If you find anything that does not make sense or if you do think it does not make sense to merge this into main, please let me know! I am also happy to answer any questions regarding code and graph! 🙂
As
ogbl-vesseluses the ROCAUC metric and we provide presampled negative training edges, I changed the code base of SEAL_OGB w.r.t. the following things:seal_link_pred.py: In addition to the rocauc function, we use a new functionevaluate_ogb_rocaucfor the ogb evaluator. I kept the old functionevaluate_aucto support non-OGB graphs as well, as these differ slightly.seal_link_pred.py: Normalization of ogbl-vessel node features.utils.py: Use negative training samples if available.Thank you,
Julian