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

Hypertree model inference #547

Open
wants to merge 6 commits into
base: costar_hyper
Choose a base branch
from

Conversation

j-varun
Copy link
Collaborator

@j-varun j-varun commented Oct 22, 2018

No description provided.

@ahundt
Copy link
Member

ahundt commented Oct 25, 2018

This really needs to be documented and parameterized. It is very hard to make sense of this, and a bunch of things seem to be hard coded.

Can we also clean this up a bit so the "inference mode" part becomes usable for validation/test?

We also have some example code of loading a model from the web at https://github.com/jhu-lcsr/costar_plan/blob/master/ctp_integration/scripts/costar_hyper_prediction.py#L48

Copy link
Member

@ahundt ahundt left a comment

Choose a reason for hiding this comment

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

needs cleanup + documentation, particularly explanation of what these classes are, why they exist, how to use them, and what options are available.

import matplotlib.ticker as ticker


class CostarHyperTreeInference():
Copy link
Member

Choose a reason for hiding this comment

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

no documentation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry about that. I'll add more documentation.


class CostarHyperTreeInference():

def __init__(self, filenames, hyperparams_json, load_weights, problem_name):
Copy link
Member

Choose a reason for hiding this comment

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

initialization of what?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add more documentation.

self.gripper_action_goal_idx = []
self.inference_mode_gen(self.filenames)

def inference_mode_gen(self, file_names):
Copy link
Member

Choose a reason for hiding this comment

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

does everything get loaded into memory at once? What happens when we are loading 128 examples, will it just run the computer right out of memory?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Everything does not get loaded at once. I'll add more comments in the code.


# print("len of X---", len(data[0]))
frame_counter += 1 % frame_len
score = model.evaluate(data[0], data[1])
Copy link
Member

Choose a reason for hiding this comment

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

won't this break with other data configurations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It shouldn't. I'll verify it anyways.

ax.set_color_cycle([plt.cm.cool(i) for i in np.linspace(0, 1, n_lines)])
count = 0
for i in indexes[1:]:
goals = self.gripper_action_goal_idx[count]
Copy link
Member

Choose a reason for hiding this comment

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

needs comments, this code is very confusing, can't parts of this be split to separate simple functions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add more comments and clean up the code.
I wanted a single function for plotting because making a function every time we add a new plot did not make sense. Anyways, I'll split it into two for now.

load_weights = "2018-09-04-20-17-25_train_v0.3_msle-vgg_semantic_rotation_regression_model--dataset_costar_block_stacking-grasp_goal_aaxyz_nsc_5-epoch-412-val_loss-0.002-val_angle_error-0.279.h5"
# filenames_updated, file_len_list = inference_mode_gen(filenames[:2])
print(len(filenames))
training_generator = CostarBlockStackingSequence(
Copy link
Member

Choose a reason for hiding this comment

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

were there any changes to the block stacking sequence class itself?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did make some changes but then changed this code to work without those changes.


if __name__ == "__main__":

filenames = glob.glob(os.path.expanduser(r'C:\Users\Varun\JHU\LAB\Projects\costar_task_planning_stacking_dataset_v0.1/*success.h5f'))
Copy link
Member

Choose a reason for hiding this comment

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

all of this needs comments + command line parameter support

@ahundt
Copy link
Member

ahundt commented Nov 14, 2018

@j-varun Hey we are in the middle of porting things to pytorch, can the keras specific parts be in a separate file? GitHub.com/ahundt/costar_dataset

@RexxarCHL we will need to bring these changes over too

@RexxarCHL
Copy link
Collaborator

Will do.

…t_generator.py to enable an easier pytorch port of inference code
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.

3 participants