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

Why newfc extend AttModel? #128

Closed
luo3300612 opened this issue Sep 20, 2019 · 7 comments
Closed

Why newfc extend AttModel? #128

luo3300612 opened this issue Sep 20, 2019 · 7 comments

Comments

@luo3300612
Copy link

Thank you for your repo, Here is my question
when I eval newfc, there is some error

tmp_att_feats = p_att_feats[k:k+1].expand(*((beam_size,)+p_att_feats.size()[1:])).contiguous()
TypeError: 'NoneType' object has no attribute '__getitem__'

and I found that newfc extends AttModel, but in initialization, we do not read att feats for newfc as in misc.utils.py

def if_use_feat(caption_model):
    # Decide if load attention feature according to caption model
    if caption_model in ['show_tell', 'all_img', 'fc', 'newfc']:
        use_att, use_fc = False, True
    elif caption_model == 'language_model':
        use_att, use_fc = False, False
    elif caption_model in ['topdown', 'adaatt','stackatt','denseatt','adaattmo']:
        use_fc, use_att = True, True
    else:
        use_att, use_fc = True, False
    return use_fc, use_att

So could you please tell me why newfc extend AttModel ?

@ruotianluo
Copy link
Owner

replace l770 in AttModel.py with return fc_feats, att_feats, att_feats, att_masks.

I fixed it locally.

@luo3300612
Copy link
Author

replace l770 in AttModel.py with return fc_feats, att_feats, att_feats, att_masks.

I fixed it locally.

Thanks for your reply. It works.
Actually I am more interested in why it extends AttModel than solving this problem.
I will appreciate it if you could tell me ~

@ruotianluo
Copy link
Owner

Basically treat fc model as a special case of attmodel where there is no attention.

So that the fc model can reuse all the functions of attmodel. Basically to reduce redundant code.

@andrewcao95
Copy link

andrewcao95 commented Mar 2, 2020

Basically treat fc model as a special case of attmodel where there is no attention.

So that the fc model can reuse all the functions of attmodel. Basically to reduce redundant code.

@ruotianluo You mean the newfc is like show tell and attmodel like show attend tell? I want to train show tell and show attend tell model as experiment result compare. Any suggestions?

#53 (comment)

@ruotianluo
Copy link
Owner

FC is what's in self. Critical sequence training paper. It's not exactly show and tell but quite similar. Same for att2in which can be viewed as a variant for show attend and tell

@andrewcao95
Copy link

FC is what's in self. Critical sequence training paper. It's not exactly show and tell but quite similar. Same for att2in which can be viewed as a variant for show attend and tell

Got it. That‘s different and will be treat as different compare methods. So, how to run train the original show tell and show attend tell ? (I think it ’s important as a baseline to compare.)

@ruotianluo
Copy link
Owner

If you really want to compare, you can try running it and fix all the errors.

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

3 participants