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

Reorganize core abstract function calls and simplify APIs #125

Open
AvinashBukkittu opened this issue Jan 15, 2020 · 4 comments · Fixed by #140
Open

Reorganize core abstract function calls and simplify APIs #125

AvinashBukkittu opened this issue Jan 15, 2020 · 4 comments · Fixed by #140
Labels
enhancement New feature or request topic: interface The design and implementation of the Forte interface topic: processors Issues related to design and development of processors

Comments

@AvinashBukkittu
Copy link
Collaborator

AvinashBukkittu commented Jan 15, 2020

Reorganize core abstract function calls and simplify APIs:

  • For processors:

    • Currently, few processors have default HParams and a few don't. I suggest the following to maintain consistency. This function should appear in base_processor but can have a default empty implementation.
      1. All processors will have the default_config method. If a processor has no parameters, this method can return {}
      2. If the user wishes to use default hyperparameters, the user should not be forced to supply it during the add_processor call to the pipeline. The pipeline should automatically use the default HParams during initialize() of that processor
      3. If the user wishes to use a non-default hyperparameter, then it can be passed through add_processor method
    • initialize function can have a default implementation and only be overwritten when needed
  • For readers:

    • cache_key_function can have a default implementation of returning None (or set the do_caching flag to false), so at this time we don't do caching.
  • For Hparams:

    • We should accept user passing configs as plain Python Dict and auto-box them to HParams.
@AvinashBukkittu AvinashBukkittu added enhancement New feature or request topic: processors Issues related to design and development of processors labels Jan 15, 2020
@gpengzhi
Copy link
Collaborator

We might also allow dict as input instead of Hparams when setting config.

@hunterhector hunterhector changed the title Using default HParams of processors Reorganize core abstract function calls and simplify APIs Jan 15, 2020
@hunterhector hunterhector added the topic: interface The design and implementation of the Forte interface label Jan 15, 2020
@gpengzhi gpengzhi self-assigned this Jan 27, 2020
@gpengzhi
Copy link
Collaborator

gpengzhi commented Jan 27, 2020

I will work on the hparams initialization part. @AvinashBukkittu @hunterhector

@gpengzhi
Copy link
Collaborator

Reopen this issue because only part of the issues is resolved.

@gpengzhi gpengzhi removed their assignment Jan 29, 2020
@hunterhector
Copy link
Member

The issue regarding default_config is solved. default_config now is a class method that can be overwritten. initialize is also to be implemented when needed. The remaining problem is cache_key_function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: interface The design and implementation of the Forte interface topic: processors Issues related to design and development of processors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants