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

Inconsistency in the structure of param_dict #351

Open
liunelson opened this issue Aug 14, 2024 · 2 comments
Open

Inconsistency in the structure of param_dict #351

liunelson opened this issue Aug 14, 2024 · 2 comments

Comments

@liunelson
Copy link

The param_dict argument of TemplateModel.set_parameters(...) has type dict[str, float] while params_dict of mira.modeling.amr.ops.add_transition(...) has type dict[str, dict[str, Any]].

It's just a minor mismatch in interface that tripped me up.

@bgyori
Copy link
Member

bgyori commented Aug 14, 2024

I see, those are different by design but I suppose it could be changed. The thinking is as follows:

  • for set_parameters, you just provide the value for each input parameter
  • for add_transition you have the option to provide an entirely new parameter definition if it appears in the new transition

Do you think set_parameters should allow creating new parameters rather than just allow updating parameter values?

@liunelson
Copy link
Author

In this case, I needed to add 6 new parameters and assign values to them. If set_parameters allows creating new parameters, then I could avoid making 6 add_parameter and 1 set_parameters calls. So yes please!

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