Skip to content
This repository was archived by the owner on Dec 16, 2022. It is now read-only.

bidirectional_language_model_transformer is registered in an odd location #2462

Closed
nelson-liu opened this issue Jan 30, 2019 · 1 comment
Closed

bidirectional_language_model_transformer is registered in an odd location #2462

nelson-liu opened this issue Jan 30, 2019 · 1 comment
Assignees
@brendan-ai2

Comments

@nelson-liu
Copy link
Contributor

Looks like BidirectionalLanguageModelTransformer is being registered as bidirectional_language_model_transformer in the __init__.py of seq2seq_encoders (

allennlp/allennlp/modules/seq2seq_encoders/__init__.py

Lines 96 to 98 in 5560466

Seq2SeqEncoder.register("bidirectional_language_model_transformer")(
BidirectionalLanguageModelTransformer
)
) .

why not just use the @Model.register("bidirectional_language_model_transformer") decorator? If there's a reason, we should include it as a comment in the __init__.py
@brendan-ai2 brendan-ai2 self-assigned this Mar 22, 2019
@brendan-ai2
Copy link
Contributor

I don't think so. I'll clean it up!

@brendan-ai2 brendan-ai2 mentioned this issue Mar 28, 2019
Merged
brendan-ai2 added a commit that referenced this issue Mar 28, 2019
@brendan-ai2
Move register to typical location. (#2662)
a80aac7 
- Fixes #2462.
reiyw pushed a commit to reiyw/allennlp that referenced this issue Nov 12, 2019
@brendan-ai2 @reiyw
Move register to typical location. (allenai#2662)
d07556f 
- Fixes allenai#2462.
TalSchuster pushed a commit to TalSchuster/allennlp-MultiLang that referenced this issue Feb 20, 2020
@brendan-ai2 @TalSchuster
Move register to typical location. (allenai#2662)
62b1914 
- Fixes allenai#2462.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@brendan-ai2 brendan-ai2

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nelson-liu @brendan-ai2