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.

Sign up for GitHub

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

Jump to bottom

Extend validator to check if all required parameters are present in parameter table #43

Closed
dweindl opened this issue Feb 4, 2019 · 6 comments · Fixed by #173
Closed

Extend validator to check if all required parameters are present in parameter table #43

dweindl opened this issue Feb 4, 2019 · 6 comments · Fixed by #173
Assignees
@dweindl
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@dweindl
Copy link
Member

dweindl commented Feb 4, 2019 •
edited
Loading

See #42 for what to check.

Should be added to petab/lint.py:

def check_parameter_sheet(problem):

EDIT: Also check that there are no additional parameters
@dweindl dweindl added enhancement New feature or request help wanted Extra attention is needed labels Feb 4, 2019
@dweindl dweindl added this to the DCM virtual hackathon 28.01.-10.02. milestone Feb 4, 2019
@elbaraim
Copy link
Collaborator

do we want to also check this with the SBML model? Meaning that all parameters defined in the SBML are present in the parameter sheet?

@dweindl
Copy link
Member Author

dweindl commented Feb 11, 2019

All besides the ones appearing as column names in the condition table, yes.

dweindl added a commit that referenced this issue Feb 18, 2019
@dweindl
Add assert_model_parameters_in_condition_or_parameter_table (#43)
ba86611 
Model non-placeholder model parameters must be either specified in the
    condition or parameter table, unless they are AssignmentRule target, in
    which case they must not occur in either.
    Check that.
dweindl added a commit that referenced this issue Feb 18, 2019
@dweindl
Add assert_model_parameters_in_condition_or_parameter_table (#43)
2ec38e2 
Model non-placeholder model parameters must be either specified in the
    condition or parameter table, unless they are AssignmentRule target, in
    which case they must not occur in either.
    Check that.
@dweindl
Copy link
Member Author

dweindl commented Feb 18, 2019

Added first part. Check for parameter overrides from measurement table still missing.

dweindl added a commit that referenced this issue Feb 18, 2019
@dweindl
Add assert_model_parameters_in_condition_or_parameter_table (#43) (#62)
3a84e84 
Model non-placeholder model parameters must be either specified in the
    condition or parameter table, unless they are AssignmentRule target, in
    which case they must not occur in either.
    Check that.
@erikadudki
Copy link
Contributor

@dweindl has this been addressed sufficiently?

@dweindl
Copy link
Member Author

dweindl commented Sep 25, 2019

@dweindl has this been addressed sufficiently?

No, at least not by me or by any commit referencing this issue.
What needs to be checked here is that all parameters introduced in the condition table (as value, not column - i.e. parameter overrides) are present in the parameter table.

@dweindl
Copy link
Member Author

dweindl commented Nov 28, 2019

See also LeonardSchmiester/Benchmark-Models#4 (comment). This is not sufficiently checked yet

@dweindl dweindl self-assigned this Dec 1, 2019
dweindl added a commit that referenced this issue Dec 1, 2019
@dweindl
Extend validator to check if all required parameters are present in p…
e2ef2a1 
…arameter table (Closes #43)
@dweindl dweindl mentioned this issue Dec 1, 2019
Merged
dweindl added a commit that referenced this issue Dec 2, 2019
@dweindl
Extend validator to check if all required parameters are present in p…
eca81a5 
…arameter table (Closes #43) (#159)
This was referenced Dec 4, 2019
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dweindl dweindl

Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Release 0.0.0a17 PEtab-dev/PEtab
Release 0.0.0a17 PEtab-dev/PEtab
3 participants
@elbaraim @dweindl @erikadudki