Conversation
swong3-sc
requested review from
kmontemayor2-sc,
mkolodner-sc,
nshah-sc,
svij-sc,
xgao4-sc,
yliu2-sc and
zfan3-sc
as code owners
kmontemayor2-sc
left a comment
kmontemayor2-sc
left a comment
There was a problem hiding this comment.
Hi Sam, thanks for the work! Left some comments.
Additionally, shall we add some basic tests for the model? IIRC you did some validation with hard-coded embeddings, maybe we should codify them so this doesn't break in the future :)
swong3-sc
left a comment
swong3-sc
left a comment
There was a problem hiding this comment.
Added tests for LightGCN (homogenous case)
kmontemayor2-sc
left a comment
kmontemayor2-sc
left a comment
There was a problem hiding this comment.
Thanks Sam! I think we're almost done here :)
kmontemayor2-sc
left a comment
kmontemayor2-sc
left a comment
There was a problem hiding this comment.
Generally LGTM provided we add a test with provided anchor nodes and update the comments.
swong3-sc
left a comment
swong3-sc
left a comment
There was a problem hiding this comment.
Added anchor test node (not for correctness, but for functionality, as we already test correctness)
Added correctness test against my math, in addition to PyG result.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
/help |
GiGL Automation@ 22:33:59UTC : 🤖 Available PR CommandsYou can trigger the following workflows by commenting on this PR:
💡 Usage: Simply comment on this PR with any of the commands above (e.g., ⏱️ Note: Commands may take some time to complete. Progress updates will be posted as comments. |
|
/unit_test |
|
/integration_tests |
GiGL Automation@ 22:34:27UTC : 🔄 @ 23:18:12UTC : ✅ Workflow completed successfully. |
GiGL Automation@ 22:34:34UTC : 🔄 @ 23:30:31UTC : ✅ Workflow completed successfully. |
|
/e2e_test |
GiGL Automation@ 22:34:43UTC : 🔄 @ 23:43:27UTC : ✅ Workflow completed successfully. |
3 participants
Scope of work done
Where is the documentation for this feature?: N/A
Did you add automated tests or write a test plan?
Yes, added unit tests
Updated Changelog.md? NO
Ready for code review?: NO