-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix dgfs #449
Fix dgfs #449
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am getting and error where no [dgf.fixed_values]
block is specified in the priors.
Traceback (most recent call last):
File "/home/georg/.virtualenvs/maud-branch/bin/maud", line 8, in <module>
sys.exit(cli())
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
return self.main(*args, **kwargs)
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/click/core.py", line 1078, in main
rv = self.invoke(ctx)
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/click/core.py", line 1688, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/click/core.py", line 783, in invoke
return __callback(*args, **kwargs)
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/maud/cli.py", line 166, in simulate_command
click.echo(do_simulate(data_path, output_dir, n))
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/maud/cli.py", line 193, in do_simulate
stanfit = simulate(mi, samples_path, n)
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/maud/running_stan.py", line 152, in simulate
set_up_output_dir(output_dir, mi)
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/maud/running_stan.py", line 191, in set_up_output_dir
cmdstanpy.utils.write_stan_json(input_path_train, mi.stan_input_train)
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/pydantic/main.py", line 756, in __getattr__
return super().__getattribute__(item) # Raises AttributeError if appropriate
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/maud/data_model/maud_input.py", line 41, in stan_input_train
train, _ = get_stan_inputs(
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/maud/getting_stan_inputs.py", line 45, in get_stan_inputs
fixed_param_input = get_fixed_param_input(parameters, kinetic_model)
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/maud/getting_stan_inputs.py", line 79, in get_fixed_param_input
"ix_dgf_free": list(range(1, len(dgf.location) + 1)),
File "/home/georg/.virtualenvs/maud-branch/lib/python3.10/site-packages/pydantic/main.py", line 759, in __getattr__
raise AttributeError(f'{type(self).__name__!r} object has no attribute {item!r}')
AttributeError: 'Dgf' object has no attribute 'location'
```
Thanks - I think this is fixed by ba18f98 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs a rebase, otherwise LGTM!
@computed_field | ||
def fixed_ids(self) -> Optional[List[List[str]]]: | ||
"""Set the fixed_ids field.""" | ||
if self.name != "dgf": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, this would better modeled with an extra class that inherits from MaudParameter
that requires implementing these two methods. I think we can leave that for the next time we want to fix a different kind of parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah there's definitely lots of room for improvement on how this is implemented, especially the inits and stan code generation. I think it'll be nice to think about this between now and when we want to generalise to other (all?) parameters.
This change partially implements #442, making it possible to fix the values of formation energy parameters in the parameter input file.
The interface is as follows for independent dgf input:
or like this for multivariate input:
I think this change is just about ready~~, the only thing to do is update the documentation~~.
Checklist: