search

Benchmarking-Initiative / Benchmark-Models-PEtab

A collection of mathematical models with experimental data in the PEtab format as benchmark problems in order to evaluate new and existing methodologies for data-based modelling
BSD 3-Clause "New" or "Revised" License
26 stars 14 forks source link

Fix Alkan_SciSignal2018 #129

Closed dilpath closed 2 years ago

dilpath commented 2 years ago

Resolves errors of this kind when running the linter.

Traceback (most recent call last):
  File "venv/bin/petablint", line 8, in <module>
    sys.exit(main())
  File "venv/lib/python3.8/site-packages/petab/petablint.py", line 164, in main
    ret = petab.lint.lint_problem(problem)
  File "venv/lib/python3.8/site-packages/petab/lint.py", line 764, in lint_problem
    check_measurement_df(problem.measurement_df, problem.observable_df)
  File "venv/lib/python3.8/site-packages/petab/lint.py", line 177, in check_measurement_df
    measurements.assert_overrides_match_parameter_count(
  File "venv/lib/python3.8/site-packages/petab/measurements.py", line 314, in assert_overrides_match_parameter_count
    actual = len(split_parameter_replacement_list(
  File "venv/lib/python3.8/site-packages/petab/measurements.py", line 241, in split_parameter_replacement_list
    return list(map(convert_and_check, result))
  File "venv/lib/python3.8/site-packages/petab/measurements.py", line 236, in convert_and_check
    raise ValueError(
ValueError: The value '' in the parameter replacement list 'scale_patm_au;' is neither a number, nor a valid parameter ID.
elbaraim commented 2 years ago

Thanks @dilpath for the fix! I have however noticed some measurements with empty values in the measurement column and they seem to be repeated. e.g.

pDNAPK_au   model1_data22   0.25 scale_pdnapk_au sd_pdnapk_au model1_data22_pdnapk_au

I guess those can be removed since they seem to be duplicated entries @LaraFuhrmann any thoughts?

dilpath commented 2 years ago

Thanks @dilpath for the fix! I have however noticed some measurements with empty values in the measurement column and they seem to be repeated. e.g. pDNAPK_au model1_data22 0.25 scale_pdnapk_au sd_pdnapk_au model1_data22_pdnapk_au

I guess those can be removed since they seem to be duplicated entries @LaraFuhrmann any thoughts?

Good catch! Not all are duplicates. https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab/blob/4aa5e4fce9d23910b043791ada58bc371c425618/Benchmark-Models/Alkan_SciSignal2018/measurementData_Alkan_SciSignal2018.tsv#L1618

elbaraim commented 2 years ago

shall we just remove the repeated empty entries? even the datasetId is identical those that are not repeated, but still empty we can keep just to be safe

dilpath commented 2 years ago

shall we just remove the repeated empty entries? even the datasetId is identical those that are not repeated, but still empty we can keep just to be safe

Sure, should be done now. Would be great to get feedback from @LaraFuhrmann when possible, but will merge now if approved.

elbaraim commented 2 years ago

shall we just remove the repeated empty entries? even the datasetId is identical those that are not repeated, but still empty we can keep just to be safe

Sure, should be done now. Would be great to get feedback from @LaraFuhrmann when possible, but will merge now if approved.

@dilpath could you also correct the simulatedData file? also there the semicolons are present and the weird duplicated 😓

EDIT: also the index column could be removed in the simulatedData

LaraFuhrmann commented 2 years ago

Thanks for taking care of that. I do not recall any reason for the empty (or duplicate) measurement cells.

dilpath commented 2 years ago

Thanks for taking care of that. I do not recall any reason for the empty (or duplicate) measurement cells.

Thanks for the feedback! Currently only "duplicated and empty" measurements are removed ("unique and empty" measurements are kept, not sure what they are for though).

shall we just remove the repeated empty entries? even the datasetId is identical those that are not repeated, but still empty we can keep just to be safe

Sure, should be done now. Would be great to get feedback from @LaraFuhrmann when possible, but will merge now if approved.

@dilpath could you also correct the simulatedData file? also there the semicolons are present and the weird duplicated sweat

EDIT: also the index column could be removed in the simulatedData

Thanks, done now.

elbaraim commented 2 years ago

sorry for disturbing, but i have noticed as well that some exp. conditions are duplicated 😓 e.g. conditions with all inputs set to zero ... model1_data45, model1_data59 ...