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

Enable newer Python versions and use of own test data #9

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

NinaWie
Copy link

@NinaWie NinaWie commented Nov 15, 2022

Hi, thanks for providing this nice codebase! I am using it for an own project, and I did some modifications to the code in order to run it with later Python versions, and in order to use my own data. Maybe you want to consider merging my new version into your repository :) Here's a summary of the changes

  • Change Set to set, cPickle to pickle, and xrange to range to enable Python 3 (I'm using 3.9.5)
  • Add test data in geojson format to simplify the use of own data (compared to the pickled tuple-objects that are used here, geojson is much easier to use and more standard)
  • Add a test script to use a trained model to get the embedding for new data. To enable this test script, I needed to make some changes to the Trainer class and to the model.py file. Maybe I overlooked some other option to do this; please let me know if that changes are redundant.
  • Explaination of the own-data usage in README

# Save parameters
config = vars(args)
with open(os.path.join(args.model_dir, "config.json"), "w") as outfile:
json.dump(config, outfile)
Copy link
Author

Choose a reason for hiding this comment

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

I added this in order to improve model loading, compared to the long file name with all parameters in the file name.

if do_full_eval == False:
# random sample each context points in NeighborGraph()
self.sample_neg_pts(ng_list)

center_pred_embed = self.predict(ng_list)
Copy link
Author

Choose a reason for hiding this comment

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

In this file, I added a predict function for each model, which only embeds the point itself and not the positive and negative points. The predict function is then used in the forward functions. This looks like a lot of changes but it's mainly encapsulation of the center_pred_embed part

@@ -240,6 +240,136 @@ def make_args_combine(args):
return args_combine


def make_enc_dec(args, pointset=None, feature_embedding= None):
Copy link
Author

Choose a reason for hiding this comment

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

This is not new code; it's only an encapsulation of code that was previously part of the __init__ of the Trainer class. In the new version, it is a separate function, and can be used in the test.py script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant