alan-turing-institute / grace

Graph Representation Analysis for Connected Embeddings
Other
34 stars 0 forks source link

Simplify `run.py` 🏃‍♀️ #308

Closed KristinaUlicna closed 10 months ago

KristinaUlicna commented 11 months ago

PR contribution summary

Why is this PR useful / good for? Please describe the problem(s) you're trying to address.

List of proposed changes / linked issues & discussions

What should a reviewer concentrate their feedback on?

What type of PR is this? (check all applicable)

Added tests?

Hints for the reviewer:

At review time, please try a bunch of files which do not have the node features & edge properties in them. Set the store_graph_attributes_permanently config hyperparameter to False 👇

store_graph_attributes_permanently: False

which should throw an error & instructions on what to do. Please follow the instructions until you get to the point where you can successfully train your model 😃


PR review summary

Describe what this PR does & how you reviewed the individual items, where needed:

Some helper checks to tick off:

In conclusion, after my review, I'd like to:

review-notebook-app[bot] commented 11 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

crangelsmith commented 11 months ago

Question:

If I already have the graph attributes saved in my graph but I want to update them I should also set: store_graph_attributes_permanently: True?

In that case some suggestions:

KristinaUlicna commented 10 months ago

After some offline discussions & design re-consideration, this PR lost on value as the issues can be approached differently, without needing to store the NODE_FEATURES vector in the written-out graph. This gets addressed in #329 . Closing this deprecated PR now.