-
Notifications
You must be signed in to change notification settings - Fork 5
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
export to rdf #54
export to rdf #54
Conversation
update local branch with changes from main
* missing probeye.txt (#56) * explicitly added probeye.txt in setup.cfg * increased version numbers * post-processing (#30) * revised arviz-based post-processing in create_posterior_plot, create_trace_plot * applied black to manually merged sampling.py * fixed figure size of one of the plot types * fixed some problems with the three plot-types * adjusted version number
…into export_to_rdf
…into export_to_rdf
Most of the rdf export work is done at this point. This refers to the export of the problem definition. The inference data is not yet part of the export-framework. However, the export methods (and frankly also some other methods) are not well tested yet. So I'm gonna work on the testing framework now. |
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 will separately look into the owl first, but there are general questions related to the implementation (that is why I did not review everything yet). In addition, the RDF export seems to be mixed up with other changes.
@@ -150,13 +150,15 @@ def response(self, inp: dict) -> dict: | |||
problem.add_parameter( | |||
"std_noise", | |||
"likelihood", | |||
domain="(0, +oo)", |
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.
looks like w weird composition of +oo. Even though I understand this, wouldn't it be better to pass a (lambda) function that evaluates to true (in the domain) or false outside. Thus, it would be much more flexible than pure intervals.
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.
True. But this is the definition part of the problem where computing methods should be avoided. Providing a specific function might also lead to problems between numpy.arrays and torch.tensors when used by the solvers. The advantage of intervals is their simplicity and therefore easier definition in the ontology. Most application cases should be covered by the interval-option.
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.
There are some applications, where the constraints actually include more than one parameter (e.g. they should be inside circle, on one side of a hyperplane, ..). That is tricky to model and requires additional effort. If it is just a function of the parameter dict (that you yould also provide/document (e.g. https://stackoverflow.com/questions/40828921/parsing-a-string-input-into-a-lambda-function-python) as a string, it is much less code, much easier to extend by someone and thus easier to maintain. As for the pyro, we we should have a short discussion.
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 think this can be treated as an extension that is mostly independent from this PR. I opened a new issue #73 for that feature.
# this is the formula for the variance of the sum of two normal dist. | ||
var = prms["std_model"] ** 2 + prms["std_measurement"] ** 2 | ||
else: | ||
var = prms["std_model"] ** 2 |
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.
is that the multiplicative case??
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.
No, the additive case. See here.
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.
so what is that std['model'] not sure that I understand that. I thought in the current implementation, the model uncertainty is zero. What exactly is that test for a the model std to be positive this should never be negative (I mean by definition)
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.
The model error is meant here as the error between the model response and the corresponding experimental data. A more elaborate name is the "model prediction error". The checks have been defined before parameter domains have been introduced, because it was possible that the sampler were proposing negative values for std-parameter, which would then lead to an error.
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.
what are std_model and std_measurement?
@@ -2187,8 +2164,10 @@ def loglike( | |||
if std_meas <= 0: | |||
return worst_value | |||
else: | |||
# consistent with tripy interface | |||
std_meas = None | |||
# in case of zero-residuals, a value of std_meas = 0 leads to a covariance |
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.
why is that the residuals and not the value of the function
t1 = iri(peo.single_experiment_data_set(exp_name)) | ||
t2 = RDF.type | ||
t3 = iri(peo.single_experiment_data_set) # type: Union[URIRef, Literal] | ||
graph.add((t1, t2, t3)) |
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.
Not sure if you copied that from van Dung, but I had the impression that owlready allows to instantiate obejcts and only afterwards export to the graph. Here, you directly add all the triples directly.
https://owlready2.readthedocs.io/en/latest/class.html#creating-individuals
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.
Yes, I add the triples directly because I think this is most transparent. The add-function is also not an owlready2-function but is defined in rdflib, see here.
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.
The ontology is quite complex, and I just started to look into the class definitions. From my point of view, it might be possible to remove some of the classes reducing complexity and allowing others to better understand the concepts.
|
||
<!-- https://www.parameter_estimation_ontology.org#Gaussian_likelihood_model --> | ||
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#Gaussian_likelihood_model"> |
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.
Why do we need to define all classes separately, why not e.g.
likelihood_model has_distribution distribution
distribution is_type normal
distribution has_correlation correlation_function
correlation_function depends_on time
correlation_function depends_on space
|
||
<!-- https://www.parameter_estimation_ontology.org#array_constant --> | ||
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#array_constant"> |
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.
why do we need to define a new class constants?
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#constant"> | ||
<rdfs:subClassOf rdf:resource="https://purl.obolibrary.org/obo/IAO_0000029"/> | ||
<obo:IAO_0000115 xml:lang="en">A numeral that specifies the number(s) it denotes by their value(s).</obo:IAO_0000115> |
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.
explanation not clear
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#correlation_model"> | ||
<rdfs:subClassOf rdf:resource="https://www.parameter_estimation_ontology.org#mathematical_function"/> | ||
<obo:IAO_0000115 xml:lang="en">A mathematical function that models correlation.</obo:IAO_0000115> |
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.
is that really a function?
|
||
<!-- https://www.parameter_estimation_ontology.org#matrix_constant --> | ||
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#matrix_constant"> |
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.
isn't being constant rather a property than a new class?
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#probability_distribution_function"> | ||
<rdfs:subClassOf rdf:resource="https://www.parameter_estimation_ontology.org#mathematical_function"/> | ||
<obo:IAO_0000115 xml:lang="en">A mathematical function that results in some measure related to probability.</obo:IAO_0000115> |
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.
that is not precise, for a mathematical function I would expect the definition of the input space and the output space.
|
||
<!-- https://www.parameter_estimation_ontology.org#sensor --> | ||
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#sensor"> |
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.
Does that mean all variables that are constants are sensors?
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#single_experiment_data_set"> | ||
<rdfs:subClassOf rdf:resource="https://purl.obolibrary.org/obo/IAO_0000100"/> | ||
<obo:IAO_0000115 xml:lang="en">A data set that is an aggregate of data recordings all of which have in common that they have been measured in a single experiment.</obo:IAO_0000115> |
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.
from my point of view, it is really difficult to define objectively, what a single experiment is. If you perform a test series (an experiment) on 5 different materials and perform 3 different tests each and use 5 specimens extracted from one bulk material (of the 5 materials), what is the number of experiments?
|
||
<!-- https://www.parameter_estimation_ontology.org#triple_constant --> | ||
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#triple_constant"> |
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.
the name is not very intuitive, and I'm also not sure why this is needed?
|
||
<!-- https://www.parameter_estimation_ontology.org#uninformative_probability_density_function --> | ||
|
||
<owl:Class rdf:about="https://www.parameter_estimation_ontology.org#uninformative_probability_density_function"> |
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.
Does an uninformative pdf (as a term) really exist, or is it in the implementation rather a specific distribution with specials choices on the parameters to make it noninformative. (I guess the term is rather noninformative than uninformative)
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's an own thing. The term "uninformative prior" is used for example here.
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.
The term is used in the literature, but at the end it is always a specific distribution behind it. IMO when creating the distribution, there could be a class that creates a non-informative distribution (e.g. as an argument), the return value should however be the precise distribution that has been choosen. (Otherwise the description is not reproducible)
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.
So the uninformative prior is only relevant for maximum likelihood fits. An uninformative prior is equivalent with no prior defined. When performing a maximum likelihood fit, the user does not have to specify a prior. Internally, this is handled as an uninformative prior.
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 would not fully agree. If you perform maximum likelihood, the prior is not used at all, i.e. it should not matter what distribution you are using. If you perform a full Bayesian approach, you have to specify a prior with a distribution function. There are different options discussed in literature, but they all have an underlying (flat) distribution that in particular depends on the scaling of the variable (so what could be considered a non-informative prior of a parameter x, it might not be a non-informative prior for a variable scaled by some factor e.g. y=1e6-6 x (e.g. when changing from Pa to MPa). In bayem, they used shape=1.0e-6, scale=1e6 for a non-informative Gamma distribution.
In this branch, probeye's export-to-rdf feature should be developed. This feature should allow to export a well-defined inference problem to an rdf-file containing data triples relating to an ontology for parameter estimation problems (see probeye/parameter_estimation_ontology.owl), that describe the problem unambiguously.