Observable/Noise parameter naming #594
Comments
|
I think, the suffix was intended to reduce the chances of collision with already exiting parameters with a different purpose. (Not saying that it would be a good idea to name your parameters I am open to changing that, but I would prefer not just dropping the suffix, but instead, introducing a column Concrete example: Current:
Proposed:
I don't think so. It will not complain about the name as such, but it will not consider it as a placeholder. Whether that results in an error will depend on the rest of the petab files. (If not, please post an example/issue). (EDIT: Okay, I think you were referring to Benchmarking-Initiative/Benchmark-Models-PEtab#261) |
Well if it's only about collisions they could just universally be renamed to |
In principle, yes. (Or even shorter, However, the problem remains, that those IDs aren't very descriptive and that certain IDs have special meaning. If we change the way these placeholders are handled/specified, I'd prefer directly addressing these issues as well. |
Sounds reasonable. How about using non-PEtab IDs, i.e. |
It would definitely be more descriptive. However, I'm not so sure about the simpler parsing. I could imagine that not every symbolic engine will allow dots in their symbols. (Could use |
|
Another suggestion is to allow arbitrary columns like Overall this is much nicer for the user and for tools I guess, but arbitrary columns are suboptimal. Example of a single problem with two measurement tables, one per observable Measurement table 1
Measurement table 2
Observable table
I split this into two measurement tables, since |
|
Another suggestion is to support labels in the Measurements table
Observables table
|
|
Thanks for the suggestions. What would be important for me when we change the current measurement-specific parameters approach:
Seeing directly from the measurement table which parameters are overridden would be nice and more robust, but is probably inconvenient to write. This would also be inconsistent with other cases, such as priorParameters, where we have lists of parameters. Either keyword parameters everywhere or nowhere, but that's probably a separate discussion. |
I'm fine with this, but: it's not evident which parameters are experiment/condition-specific, so why care about measurement-specific parameters? To make life a bit easier for tools? Apart from that I agree on all points. So I would vote first for your |
For me there is a slight difference: other parameters may be overridden in a condition-specific way, but those placeholders must be overridden for any measurements of the respective observable. (At least, that is how it used to be.) Therefore, I'd prefer seeing easily which ones have to be specified with the measurements. |
|
I definitely like the suggestion by @dweindl much better then what we have right now.
|
|
@FFroehlich @fbergmann @sebapersson : Do you have any thoughts on this? Keep it as it was in v1? Change? (How?) |
I agree on this. This is something that should be straightforward to parse for an importer. |
|
Follow-up question: would the placeholder parameter IDs have to be globally unique or would the following be allowed (
At first glance, it might be confusing that |
|
@dweindl Just for clarification, if I wanted to use other model elements in the observableFormula, that would be fine, right? And only if tokens from the observableFormula are not in the model, they would be sought in the placeholders column. Right? As for uniqueness, in v1, things were unique by default, since we forced the suffix. As such, even without reading and memorising the spec, you would see the parameters belonging together. in V2, looking at the example above, I would then have to go to read / remember the spec, to recall that scale would always be different for obs1 and ob2 if used as in the example. |
Yes, anything from the model file or the parameters table could be used there.
I would put it the other way around: If there is anything specified in the placeholders column, then the values of those entities needs to be retrieved from the measurement table row for the the current timepoint/experiment/observable.
Yes and no. I think in terms of uniqueness it's similar to different simulation conditions (v1) or different experiments (v2), where the same parameter may take different values. Those overrides from the measurement table are kind of micro-conditions. The condition-specific parameters aren't evident from the model. However, here, the placeholder parameters are specified as such just next to where they are used. So I find it even more explicit here. I don't have a very strong opinion on this matter. If it makes life significantly easier, I am also fine with requiring globally unique IDs. Just be explicit about the example above: That would assume/require that |
|
If we separate the IDs into two groups "PEtab IDs" and "placeholder IDs" (IDs in the But I prefer globally unique IDs, because I don't think it's completely correct to view overrides as micro-conditions. In PEtab conditions, the parameter that gets overridden with different values always has the same meaning: "this is the rate constant for reaction R". But in this case, we cannot say "this is the offset for observable O", rather just "this is the offset ID for all observables". The offset for each observable seems different conceptually, for example, each offset will have different units. |
Fair. I mixed the measurement-specific parameters in general with the re-use of parameter IDs. It only applies to the former. |
|
I would prefer globally unique ids. This makes things often easier in the implementation and avoids confusion (at the cost of having to make the ids unique). For instance storing things in hash maps is so much easier with unique ids instead of having to create your own unique ids for objects. |
|
What's the point of having globally unique placeholders? Apart from time-point specific overrides (which is rare, as far as I know) everything else could be achieved by overriding scales/offsets in a condition specific manner, right? |
It might be more convenient for annotations (e.g., units).
Yes, timepoint-specific output parameters are the main reason for those placeholders. Are you suggesting to drop that completely? |
Fair.
Not sure, perhaps. With the new conditions/experiments spec, you could just take the "micro-condition" approach and add a ton of conditions that update the respective parameters, so there is not really any new functionality that we are supporting with this. Thus, we should be mainly concerned about how much this simplifies problem formulation and facilitates conceptual understanding of the spec. Introducing additional columns and introducing constraints about global uniqueness seem more geared towards formal correctness/ease of implementation rather than making it easier for the user. |
|
I agree, we should keep this as simple as possible for the user. |
|
So far, I am more in favor of keeping this in some form. It's true that some of the current uses could be replaced by condition-specifc parameters and that the remaining use cases don't seem to be super frequent, but nevertheless, they are there. I also find it preferable to have the measurement-specific parameter changes specified together with the measurements, but that's just a matter of taste 8000 (cohesion vs. conciseness), I guess. My feeling is that the simplification of the standard by removing placeholders is not that significant. If a user doesn't need it, they can just ignore it. There isn't much coupling to other parts of PEtab. The same for tool developers - they can just choose not support it (initially). (Having explicit mark up for those placeholders as proposed above would make it easier to recognize the use of that feature). |
|
@dweindl perfect. I have no preference either way. |
Currently, noise and observable parameter placeholders require
_${observableId}as suffix. This suffix appears to be redundant and superfluous to me, as they need to be specified in the measurements table and in the measurements table you can only specify the parameter for the same observable.I can see how adding the suffix could make it more explicit that these variables are different across observables, but that should already by evident from the fact that they have to be specified in the measurements table. Neither the linter nor simulators appear to check for correctness anyways (see https://github.com/Benchmarking-Initiative/Benchmark-Models-PEtab)
The text was updated successfully, but these errors were encountered: