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

Is there a typo in the names of the processed instructions? #19

Open
TopCoder2K opened this issue Aug 9, 2022 · 6 comments
Open

Is there a typo in the names of the processed instructions? #19

TopCoder2K opened this issue Aug 9, 2022 · 6 comments

Comments

@TopCoder2K
Copy link

There are no files with names '*test_seen_appended.p' and '*test_unseen_appended.p' in models/instructions_processed_LP/, although it's clear that read_test_dict tries to load files with 'appended' in names.

@TopCoder2K
Copy link
Author

TopCoder2K commented Aug 9, 2022

Also, I want to mention some other strange places that I've encountered. Maybe someday they will be fixed.

  1. What is the purpose of this and this code? :)
  2. Why is this argument set to true? GT anything can't be used during testing.
  3. RGB images are saved incorrectly as they are in BGR format
  4. Saved FMM map is mirrored (or rotated?)
  5. The semantic policy model (UNetMulti) is always loaded from best_model_multi.pt but in the tutorial its best performing checkpoint is named new_best_model.pt (moreover, there is a typo in the tutorial since the mv's first argument is best_model_multi.pt model, not new_best_model.pt)

@soyeonm
Copy link
Owner

soyeonm commented Aug 10, 2022

Hello,

Thank you for your keen eyes, @TopCoder2K !
I uploaded the '*test_seen_appended.p' and '*test_unseen_appended.p' in models/instructions_processed_LP/.

With regards to your questions 1~5,

(general answer: A lot of engineering went into this work; I tried to clean the code but there are still some remnants of it.. sorry. )

  1. They are some remnants of engineering - trying different thresholds and numbers in different circumstances. There is no purpose.

  2. Sorry for confusing you with this. This is another remnant of engineering, but you can ignore this. If you go through the code, you will notice that "args.ground_truth_segmentation" is not used for almost anything (and definitely not for segmentation at all). I just deleted this line.

3-4. For these, users can customize it as they want.

  1. "best_model_multi.pt" is the semantic policy model used in the paper, and the "new best model" is an even better model that we found with another starting seed after submitting the paper. Users can use what they want for this as well.

@soyeonm
Copy link
Owner

soyeonm commented Aug 10, 2022

And thank you very much, @TopCoder2K !

@TopCoder2K
Copy link
Author

TopCoder2K commented Aug 10, 2022

@soyeonm, thank you for your fast response!

As for 5, should the README file be updated then? The command

mv Pretrained_Models_FILM/best_model_multi.pt models/semantic_policy/new_best_model.pt

should be

mv Pretrained_Models_FILM/new_best_model.pt models/semantic_policy/best_model_multi..pt

since the model is always loaded from best_model_multi.pt, isn't it?

And if the README gets fixed, I also want to remind you about this issue.

@soyeonm
Copy link
Owner

soyeonm commented Aug 10, 2022

@TopCoder2K , thanks very much once again. I changed the readme.

With regards to this issue, I will reply again soon! But I do believe it does work.. does it not?

@TopCoder2K
Copy link
Author

With regards to #2 (comment) issue, I will reply again soon! But I do believe it does work.. does it not?

The #9 (comment) states that it's necessary to use --data data/json_2.1.0 when running python models/train/train_seq2seq.py, not --data data/json_feat_2.1.0 as in README. And a 'thumbs up' from another user suggests that this solves the problem. Could you please check the solution and, if it is correct, update the README once again?

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

No branches or pull requests

2 participants