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.

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

[Merged by Bors] - Introduction of SamplingContext: keeping it simple #259

Closed
wants to merge 53 commits into from

Conversation

torfjelde
Copy link
Member

This is #253 but the only motivation here is to get SamplingContext in, nothing relating to interactions with other contexts, etc.

@@ -39,3 +39,6 @@ Possibly existing indices of `varname` are neglected.
) where {s,missings,_F,_a,_T}
return s in missings
end

# HACK: Type-piracy. Is this really the way to go?
AbstractPPL.getsym(::AbstractVector{<:VarName{sym}}) where {sym} = sym
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the other PR:

It's just convenient for the dot_tilde_* statements where we are working with vns::AbstractVector{<:VarName{sym}}. This way we can just use getsym as before instead of dispatching on this (which will inevitably lead to method ambiguity due to the ordering of args in tilde)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked a bit more carefully now and I think it is only needed because of

return VarName(vn, (vn.indexing..., Tuple(i)))
and similar lines - is this correct? So I think ideally these lines should be changed and only return a VarName{sym} in which the indices are mapped together (but not an array of VarName{sym}s).

However, I guess we might want to change this in a different PR since this is not newly introduced in the PR and quite different from the rest of the PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not what I had in mind actually; I was thinking of https://github.com/TuringLang/DynamicPPL.jl/blob/tor%2Fsampling-context-simple/src/context_implementations.jl#L359-L369

The getsym call here will fail since vn is now vn::AbstractArray{<:VarName}. The alternative is to extract it in the method signature, but this is somewhat annoying + can lead to method ambiguities since it's one of the latter arguments for the method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of https://github.com/TuringLang/DynamicPPL.jl/blob/tor%2Fsampling-context-simple/src/context_implementations.jl#L359-L369

This is how I understood your explanation 🙂 To me it just seemed that we only end up with AbstractArray{<:VarName{sym}} there because of the methods linked above. It seems that we do not support things like [VarName{sym1}(...), VarName{sym2}(...)] currently so I assumed this was the only reason for the arrays there. But maybe this is not the only way to get them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaah, I reread your previous comment, looked up the line you're referring to and now I see what you're saying.

But I think this is the only way actually. And regarding the assumptions it makes, I also noted the same but then thought "Does it even make sense to have dot_tilde_* implementation for cases [VarName{sym1}, VarName{sym2}]? Nah." You got anything in mind?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I do like your suggestion (if I understood correctly) of allowing VarName to contain a collection of indices. But that should prob go to APPL in a different PR.

Sooooo type-piracy while we wait for that or nah?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's go with the type piracy. Avoiding the AbstractArray{<:VarName{sym}} and changing these functions seems like the "correct" approach, but I don't think it has to be fixed in this PR and it seems a bit unrelated to all other changes here, so let's go with the type piracy and fix it later (and possibly in AbstractPPL - doesn't it already allow multiple indices? At least Colon() is supported IIRC).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to open an issue, here and/or in AbstractPPL, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add it here: TuringLang/AbstractPPL.jl#7. Maybe a there's a lattice structure or something to varnames, it kind of feels like it, then we'd have join/meet and the meet is the collection of indices.

And yes, in theory something like VarName{x}((([1, 3],),)) already works, but I don't know how stable it is wrt. subsumption. I think it's mentioned as disallowed in the docstring.

src/model.jl Outdated Show resolved Hide resolved
@torfjelde
Copy link
Member Author

This good @devmotion ?

@devmotion
Copy link
Member

Difficult to keep track off all the different iterations but it seems fine 🙂 Do you intend to address the matchingvalue comment in this PR or a subsequent one?

@torfjelde
Copy link
Member Author

Difficult to keep track off all the different iterations but it seems fine 🙂

Yeah sorry 😕 As I said, I got a bit carried a way in the previous PRs. I opened a new one because I'd like to keep the other one around (probably close it, but still useful to have lying about for when we think more about handling contexts properly). Really want a good way to deal with all the contexts + make it easy to introduce new ones, but I'll leave it for now.

Do you intend to address the matchingvalue comment in this PR or a subsequent one?

I'll address in this one (i.e. now) 👍

@torfjelde
Copy link
Member Author

bors try

@torfjelde
Copy link
Member Author

Yo why isn't bors doing anything? Is he on strike?

@yebai
Copy link
Member

yebai commented Jun 8, 2021

bors r+

bors bot pushed a commit that referenced this pull request Jun 8, 2021
This is #253 but the only motivation here is to get `SamplingContext` in, nothing relating to interactions with other contexts, etc.

Co-authored-by: Hong Ge <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jun 8, 2021

Build failed:

@torfjelde
Copy link
Member Author

I would suggest you @devmotion @yebai do another review before we merge it (this is why I did bors try). I've made some changes that simplifies stuff, e.g. removed the "unwrap child"-functionality.

@torfjelde
Copy link
Member Author

torfjelde commented Jun 9, 2021

Also, why are the tests failiing like that? Didn't we just add a mechanism for allowing the integration tests to fail if we're making a breaking release? o.O

EDIT: Ah, I see. I guess empty intersection is a PkgError, and we specifically only catch ResolveError. I guess this is useful since it means that tests will fail if we haven't bumped the version of the integration tests accordingly 👍

@torfjelde
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Jun 9, 2021
Copy link
Member

@devmotion devmotion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good mostly, I just spotted some incorrect docstrings and some minor things that were not addressed or reverted after you had applied some additional fixes. I also think that it would be more natural (and shouldn't break anything or require additional changes) to fall back to tilde_observe! and dot_tilde_observe! instead of tilde_observe and dot_tilde_observe when the variable names and indices are neglected.

src/compiler.jl Outdated Show resolved Hide resolved
src/context_implementations.jl Outdated Show resolved Hide resolved
src/context_implementations.jl Outdated Show resolved Hide resolved
src/context_implementations.jl Outdated Show resolved Hide resolved
src/context_implementations.jl Outdated Show resolved Hide resolved
Comment on lines +77 to +78
# Need the `logp` value, so we cannot defer `acclogp!` to child-context, i.e.
# we have to intercept the call to `tilde_observe!`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to be addressed in this PR but maybe it would make sense to return value + logp in the tilde pipeline, also from tilde_assume! and tilde_observe!?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but then we need to change the compiler + the logp wouldn't actually be used for anything in the model. But yeah, might still be worth just to make the tilde-pipeline a bit nicer.

src/loglikelihoods.jl Outdated Show resolved Hide resolved
src/model.jl Outdated Show resolved Hide resolved
src/model.jl Outdated Show resolved Hide resolved
@@ -0,0 +1,123 @@
# A collection of models for which the mean-of-means for the posterior should
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we wouldn't duplicate these in DynamicPPL and Turing... Maybe it would make sense to even add example models for tests to DynamicPPL (similar to test utilities for kernels in https://github.com/JuliaGaussianProcesses/KernelFunctions.jl/blob/master/src/test_utils.jl)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I 100% agree with this, but a bit uncertain of the route to take. Though I think what you've done in KernelFunctions.jl is interesting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to have some test models inside DynamicPPL, which can be then imported by Turing. Overall, I think we should gradually make sure tests in DynamicPPL are more self-contained.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we now perform integration tests with Turing (that also handle breaking and non-breaking changes correctly), maybe we can remove the remaining Turing-dependent tests in https://github.com/TuringLang/DynamicPPL.jl/tree/master/test/turing or integrate them with the other tests after removing the Turing dependency. These tests are already isolated and also don't block breaking updates anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a good idea 👍

But do we leave that for another PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I wouldn't include any additional changes here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just ran bors this merges into tor/tilde-simplification anyways, so if we indeed want to include the test-refactoring we can do that there (IMO we leave it for a subsequent PR which should be soon, since that will be very useful)

src/context_implementations.jl Outdated Show resolved Hide resolved
src/context_implementations.jl Outdated Show resolved Hide resolved
src/context_implementations.jl Outdated Show resolved Hide resolved
src/context_implementations.jl Outdated Show resolved Hide resolved
src/context_implementations.jl Outdated Show resolved Hide resolved
@@ -0,0 +1,123 @@
# A collection of models for which the mean-of-means for the posterior should
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we now perform integration tests with Turing (that also handle breaking and non-breaking changes correctly), maybe we can remove the remaining Turing-dependent tests in https://github.com/TuringLang/DynamicPPL.jl/tree/master/test/turing or integrate them with the other tests after removing the Turing dependency. These tests are already isolated and also don't block breaking updates anymore.

@torfjelde
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 9, 2021
This is #253 but the only motivation here is to get `SamplingContext` in, nothing relating to interactions with other contexts, etc.

Co-authored-by: Hong Ge <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jun 9, 2021

Build failed:

@torfjelde
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Jun 9, 2021
This is #253 but the only motivation here is to get `SamplingContext` in, nothing relating to interactions with other contexts, etc.

Co-authored-by: Hong Ge <[email protected]>
@bors bors bot changed the title Introduction of SamplingContext: keeping it simple [Merged by Bors] - Introduction of SamplingContext: keeping it simple Jun 9, 2021
@bors bors bot closed this Jun 9, 2021
@bors bors bot deleted the tor/sampling-context-simple branch June 9, 2021 13:18
yebai added a commit that referenced this pull request Jul 13, 2021
* initial stuff

* moved benchmark folder and added README

* unwrap distributions and varnames at model-level

* removed _tilde and renamed tilde_assume and others

* formatting

* updated compiler for new tilde-methods

* fixed calls to dot_assume

* added sampling context and unwrap_childcontext

* updated tilde methods

* updated model call signature

* updated compiler

* formatting

* added getsym for vectors

* Update src/varname.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fixed some signatures for Model

* fixed a method call

* fixed method signatures

* sort of fixed the matchingvalue functionality for model

* formatting

* removed redundant _tilde method

* removed left-over acclogp! that should not be here anymore

* export SamplingContext

* use context instead of ctx to refer to contexts

* formatting

* use context instead of ctx for variables

* use context instead of ctx to refer to contexts

* Update src/compiler.jl

Co-authored-by: David Widmann <[email protected]>

* Update src/context_implementations.jl

Co-authored-by: David Widmann <[email protected]>

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* added some whitespace to some docstrings

* deprecated tilde and dot_tilde plus exported new versions

* formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* minor version bump

* added impl of matchingvalue for contexts

* reverted the change that makes assume always resample

* removed the inds arguments from assume and dot_assume to stay non-breaking

* Update src/context_implementations.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* added missing sampler arg to tilde_observe

* added missing sampler argument in dot_tilde_observe

* fixed order of arguments in some dot_assume calls

* formatting

* formatting

* added missing sampler argument in tilde_observe for SamplingContext

* added missing word in a docstring

* updated submodel macro

* removed unwrap_childcontext and related since its not needed for this PR

* updated submodel macro

* fixed evaluation implementations of dot_assume

* updated pointwise_loglikelihoods and related

* added proper tests for pointwise_loglikelihoods

* updated DPPL tests to reflect recent changes

* bump minor version since this will be breaking

* formatting

* formatting

* renamed mean_of_mean_models used in tests

* bumped dppl version in integration tests

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* fixed ambiguity error

* Introduction of `SamplingContext`: keeping it simple (#259)

This is #253 but the only motivation here is to get `SamplingContext` in, nothing relating to interactions with other contexts, etc.

Co-authored-by: Hong Ge <[email protected]>

* Update src/DynamicPPL.jl

Co-authored-by: David Widmann <[email protected]>

* added initial impl of SimpleVarInfo

* remove unnecessary debug statements to be compat with Zygote

* make reconstruct slightly more generic

* added a couple of convenience constructors

* formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* small fix

* return var_info from tilde-statements, allowing impl of immutable versions

* allow usage of non-Ref types in SimpleVarInfo

* update submodel-macro

* formatting and docstring for submodel-macro

* attempt at supporting implicit returns too

* added a small comment

* simplifed submodel macro a bit

* formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fixed typo

* use bang-bang convention

* updated PointwiseLikelihoodContext

* fixed issue where we unnecessarily replace the return-statement

* check subtype in the retval

* formatting

* fixed type-instability in retval check

* introduced evaluate method for model

* remove unnecessary type-requirement

* make return-value check much nicer

* removed redundant creation of anonymous function

* dont use UnionAll in return_values

* updated tests for submodel to reflect new syntax

* moved to using BangBang-convention for most methods

* remove SimpleVarInfo from this branch

* added a comment

* reverted submodel macro to use = rather than ~

* updated SimpleVarInfo impl

* added a couple of missing deprecations

* updated tests

* updated implementations of logjoint and others

* formatting

* added eltype impl for SimpleVarInfo

* formatting

* fixed eltype for SimpleVarInfo

* updated to work with master

* changed the output structure a bit

* forgot to include src

* updated jmd files

* added some docs

* updated README

* formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: David Widmann <[email protected]>
Co-authored-by: Hong Ge <[email protected]>
yebai added a commit that referenced this pull request Dec 12, 2021
* fixed some signatures for Model

* fixed a method call

* fixed method signatures

* sort of fixed the matchingvalue functionality for model

* formatting

* removed redundant _tilde method

* removed left-over acclogp! that should not be here anymore

* export SamplingContext

* use context instead of ctx to refer to contexts

* formatting

* use context instead of ctx for variables

* use context instead of ctx to refer to contexts

* Update src/compiler.jl

Co-authored-by: David Widmann <[email protected]>

* Update src/context_implementations.jl

Co-authored-by: David Widmann <[email protected]>

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* added some whitespace to some docstrings

* deprecated tilde and dot_tilde plus exported new versions

* formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* minor version bump

* added impl of matchingvalue for contexts

* reverted the change that makes assume always resample

* removed the inds arguments from assume and dot_assume to stay non-breaking

* Update src/context_implementations.jl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* added missing sampler arg to tilde_observe

* added missing sampler argument in dot_tilde_observe

* fixed order of arguments in some dot_assume calls

* formatting

* formatting

* added missing sampler argument in tilde_observe for SamplingContext

* added missing word in a docstring

* updated submodel macro

* removed unwrap_childcontext and related since its not needed for this PR

* updated submodel macro

* fixed evaluation implementations of dot_assume

* updated pointwise_loglikelihoods and related

* added proper tests for pointwise_loglikelihoods

* updated DPPL tests to reflect recent changes

* bump minor version since this will be breaking

* formatting

* formatting

* renamed mean_of_mean_models used in tests

* bumped dppl version in integration tests

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* fixed ambiguity error

* Introduction of `SamplingContext`: keeping it simple (#259)

This is #253 but the only motivation here is to get `SamplingContext` in, nothing relating to interactions with other contexts, etc.

Co-authored-by: Hong Ge <[email protected]>

* Update src/DynamicPPL.jl

Co-authored-by: David Widmann <[email protected]>

* added initial impl of SimpleVarInfo

* remove unnecessary debug statements to be compat with Zygote

* make reconstruct slightly more generic

* added a couple of convenience constructors

* formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* small fix

* return var_info from tilde-statements, allowing impl of immutable versions

* allow usage of non-Ref types in SimpleVarInfo

* update submodel-macro

* formatting and docstring for submodel-macro

* attempt at supporting implicit returns too

* added a small comment

* simplifed submodel macro a bit

* formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fixed typo

* use bang-bang convention

* updated PointwiseLikelihoodContext

* fixed issue where we unnecessarily replace the return-statement

* check subtype in the retval

* formatting

* fixed type-instability in retval check

* introduced evaluate method for model

* remove unnecessary type-requirement

* make return-value check much nicer

* removed redundant creation of anonymous function

* dont use UnionAll in return_values

* updated tests for submodel to reflect new syntax

* moved to using BangBang-convention for most methods

* remove SimpleVarInfo from this branch

* added a comment

* reverted submodel macro to use = rather than ~

* updated SimpleVarInfo impl

* added a couple of missing deprecations

* updated tests

* updated implementations of logjoint and others

* formatting

* added eltype impl for SimpleVarInfo

* formatting

* fixed eltype for SimpleVarInfo

* implement setindex!! in prep for allowing sampling with immutable vi

* formatting

* initial work on allowing sampling using SimpleVarInfo

* formatting

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* add constructor for SimpleVarInfo using model

* improved leftover to_namedtuple_expr, fixing a bug when used with Zygote

* bumped patch version

* fixed set_flag!!

* forgot the return in the replace_returns

* bigboy update to benchmarks

* fixed some issues and added support for usage of Dict in SimpleVarInfo

* added docstring and improved indexing behvaior for SimpleVarInfo

* formatting

* dont allow sampling with indexing when using SimpleVarInfo with NamedTuple unless shapes are specified

* _setval_kernel and others are only supported by VarInfo atm

* fixed typo in comment

* added more values_as impls

* removed redundant values_from_metadata

* fixed bug in push!! for SimpleVarInfo

* forgot which branch Im on

* added handling of short defs in replace_returns and more docstrings

* fixed bug in generate_tilde introduced in a merge

* fixed a bug in isfuncdef

* fixed tests

* formatting

* uncomment mistakenly commented code

* bumped version

* updated doctests

* dont carry over bang-bang versions that we dont need for general varinfos

* Apply suggestions from @phipsgabler

Co-authored-by: Philipp Gabler <[email protected]>

* updated tests

* removed unnecessary BangBang methods

* fixed zygote rule for dot_observe

* fixed Setfield.jl + returning VarInfo bug in model-macro

* updated tests

* fixed docs

* formatting

* fixed issues when using ThreadSafeVarInfo

* fixed _pointwise_observe for ThreadSafeVarInfo

* updated ThreadSafeVarInfo

* made SimpleVarInfo compat with ThreadSafeVarInfo and added show

* added some tests for return-values of models

* formatting

* fixed doctest for SimpleVarInfo

* formatting

* removed comparison of show from doctest for SimpleVarInfo

* Update src/compiler.jl

Co-authored-by: David Widmann <[email protected]>

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* removed OrderedCollections from docs

* some additional fixes

* fixed method ambiguity and some ill-defined map

* renamed evaluate to evaluate!!

* added implementations of haskey, getindex and setindex!! for SimpleVarInfo

* formatting

* dropped redundant definition

* use getproperty instead of getindex

* fixed method-ambiguity and added some comments

* fixed docstring of SimpleVarInfo

* fixed docstrings

* fixed Project.toml for docs

* fixed docstring of canview

* fixed docstrings

* another attempt at fixing docstrings

* added a TODO comment

* remove some output from docstring of SimpleVarInfo

* fixed haskey and hasvalue for AbstractDict

* updated some comments

* updated some errors

* added sampling dot_assume for SimpleVarInfo

* added true versions of density computations to TestUtils

* added tests specific for SimpleVarInfo

* also document TestUtils

* added TestUtils to docs

* fixed setindex!! for SimpleVarInfo using AbstractDict

* added more tests

* formatting

* dont use BangBang for setall!

* revert unnecessary changes to settrans!

* revert unnecessary changes to set_flag!

* revert some changes to docstrings

* fixed some comments and docstrings

* added more convenient logjoint, logprior, and loglikelihood methods

* removed unnecessary export

* fixed export

* use the Setfield impl of getindex, etc. as default and specialize on AbstractDict

* fixed docstrings of logjoint, etc.

* Apply suggestions from code review

Co-authored-by: Philipp Gabler <[email protected]>

* fixed docstring for model

* replaced return_values by capturing return-value from tilde-statements instead

* added some tests for return-value of model

* added broadcast_foreach

* Apply suggestions from @devmotion

Co-authored-by: David Widmann <[email protected]>

* remove broadcast_foreach for now

* some fixes to ThreadSafeVarInfo

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* fixed docstrings

* forgot qualification for set

* formatting

* added comment about why we cant use MacroTools.isdef

* remove unnecessary deprecation

* udpated some docstrings

* fixed more docstrings

* make overloads of BangBang methods qualified

* remove overloading of values and instead use values_as without the type specified

* Apply suggestions from code review

Co-authored-by: David Widmann <[email protected]>

* renamed hasvalue for SimpleVarInfo to _haskey

* revert changes from previous commit

* minor version bump

* fixed sampling with ThreadSafeVarInfo

* fixed setindex!! for ThreadSafeVarInfo

* fixed eltype for ThreadSafeVarInfo wrapping a SimpleVarInfo

* fixed a test

* relax atol in serialization tests a bit

* temporarily disable Julia 1.3

* relax atol for a prior check

* Improvements to `@submodel` in #309 (#348)

* added prefix keyword argument to submodel-macro

* Apply suggestions from code review

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* converted example in docs into test

* fixed docstring

* Apply suggestions from code review

Co-authored-by: Philipp Gabler <[email protected]>

* removed redundant prefix_submodel_context def and added another example to docstring

* fixed doctests

* attempt at fixing doctests

* another attempt at fixing doctests

* had a typo in docstring

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Philipp Gabler <[email protected]>

* fixed a test case using submodel

* improved docstring according to comments by @devmotion

Co-authored-by: David Widmann <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hong Ge <[email protected]>
Co-authored-by: Philipp Gabler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants