Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inference of GCN model on unseen test data #223

Merged
merged 21 commits into from
Sep 12, 2023
Merged

Inference of GCN model on unseen test data #223

merged 21 commits into from
Sep 12, 2023

Conversation

KristinaUlicna
Copy link
Collaborator

@KristinaUlicna KristinaUlicna commented Sep 8, 2023

PR contribution summary

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

  • Assuming we have a working GCN, this PR provides a new notebook to run inference & evaluate the performance of the GCN on real, annotated data.
  • It builds a GraphLabelPredictor which updates the GraphAttrs.XXX_PREDICTION by a vector of numbers for each node and edge:
    for idx, node in G.nodes(data=True):
    prediction = [int(n_pred[idx].item()), n_probabs[idx].numpy()]
    node[GraphAttrs.NODE_PREDICTION] = prediction
    for e_idx, edge in enumerate(G.edges(data=True)):
    prediction = (int(e_pred[e_idx].item()), e_probabs[e_idx].numpy())
    edge[-1][GraphAttrs.EDGE_PREDICTION] = prediction
  • It allows node Annotation.UNKNOWN relabelling to be separated for nodes & edges.
  • This PR additionally annotates other notebooks to make sure that the flow of what is done is clearly explained.

List of proposed changes / linked issues & discussions

What should a reviewer concentrate their feedback on?

  • ✅ Scripts to check
  • 🏃 Notebooks to run (especially infer_predictions.ipynb, others only have minor documentation / comments changes)
  • 💻 Code quality
  • 📝 Everything looks OK?

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

  • 🪄 Feature
  • 🐛 Bug fix
  • #️⃣ Documentation / code annotation
  • 🔥 Performance Improvements

Added tests?

  • 🙋 no, because I need some help

PR review summary

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

Some helper checks to tick off:

  • Focus on image annotation
  • Focus on model training
  • Could any optimization be applied?
  • Is there any redundant code?
  • Are there any spelling errors?

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

  • 🙋 ask some clarifying questions
  • 🙅 suggest some specific changes

@KristinaUlicna KristinaUlicna added enhancement New feature or request methodology Building functional & diverse pipeline labels Sep 8, 2023
@KristinaUlicna KristinaUlicna self-assigned this Sep 8, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

"source": [
"from grace.io.image_dataset import ImageGraphDataset\n",
"from grace.models.feature_extractor import FeatureExtractor\n",
"from grace.evaluation.visualisation import plot_simple_graph\n",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get an error trying to run this cell:
ModuleNotFoundError: No module named 'grace.evaluation'

I think there is a missing init.py in the evaluation directory that would make this work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the __init__.py in the next commit 🚀

"metadata": {},
"outputs": [],
"source": [
"extractor_filename = \"/Users/kulicna/Desktop/classifier/extractor/resnet152.pt\"\n",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want people to be able to run this easily maybe we need to add the line to download the resnet before or add a comment to point to where this is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point! I included the instructions on how to download the ResNet locally in the training subfolder README:

### Downloading the feature extractor:
In case you decide to use a pre-trained image classifier, such as resnet-152, you can use this snippet to import the model, load the default weights & download the model:
```sh
import torch
from grace.models.feature_extractor import resnet
resnet_model = resnet(resnet_type="resnet152")
extractor_fn = "/path/to/your/feature/extractor/resnet152.pt"
torch.save(resnet_model, extractor_fn)
```

Would you like to see it added in the notebook, too?

"metadata": {},
"outputs": [],
"source": [
"grace_path = \"/Users/kulicna/Desktop/dataset/shape_stars/train\"\n",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general we should remove path to personal directories if we want these notebooks to be run easily for everyone, we could add a new variable called: input_paht = /path/to/data/ to be replaced by the user.

Also, we need to put this data somewhere and easy to download.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fully agreed, we had this problem in the examples\ folder where I replaced the path for a user-specified input:

# e.g. /Users/kulicna/Desktop/dataset/shape_squares/MRC_Synthetic_File_000.mrc
IMAGE_PATH = Path(
input(
"Enter absolute path to your file "
"(e.g. /Users/path/to/your/data/image.mrc, omit ''): "
)
)

I'm not sure how to do this from the notebook tho...

@crangelsmith
Copy link
Collaborator

Hi @KristinaUlicna, the notebook runs except for the minor comments above but I'm a bit confused about why is the inference done on a notebook and not added to the run.py script as part of the pipeline. Couldn't we just add an eval option to the config file and do this in run.py after training?

@crangelsmith
Copy link
Collaborator

Also, it would be good to visualise the resulting graph on inference, as described in #222

@KristinaUlicna
Copy link
Collaborator Author

Also, it would be good to visualise the resulting graph on inference, as described in #222

Coming up in PR #229 ! 🚀

@KristinaUlicna KristinaUlicna merged commit f530eba into main Sep 12, 2023
1 check passed
@KristinaUlicna KristinaUlicna deleted the inference branch September 12, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment