-
Notifications
You must be signed in to change notification settings - Fork 17
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
implement Base.show for core types #259
Conversation
What do you think @odunbar? One downside of this implementation, particularly for bounds, is that new, custom structs may not contain enough information. Consider e.g. the affine constraint type from examples: julia> d2 = Samples([1.0 5.0 9.0 13.0; 3.0 7.0 11.0 15.0]) # 4 samples of 2D parameter space
Samples{Float64}([1.0 5.0 9.0 13.0; 3.0 7.0 11.0 15.0])
julia> transform = (x -> 3 * x + 14)
#13 (generic function with 1 method)
julia> jac_transform = (x -> 3)
#15 (generic function with 1 method)
julia> inverse_transform = (x -> (x - 14) / 3)
#17 (generic function with 1 method)
julia> abstract type Affine <: ConstraintType end
julia> c2 = [bounded(10, 15),
Constraint{Affine}(transform, jac_transform, inverse_transform, nothing)]
2-element Vector{Constraint}:
Constraint{Bounded} with bounds (10, 15)
Constraint{Affine} with bounds (-∞, ∞)
julia> name2 = "constrained_sampled"
"constrained_sampled"
julia> u2 = ParameterDistribution(d2, c2, name2)
ParameterDistribution with 1 entries:
'constrained_sampled' with Constraint[Bounds: (10, 15), Bounds: (-∞, ∞)] over distribution Samples{Float64}([1.0 5.0 9.0 13.0; 3.0 7.0 11.0 15.0]) Should I explicitly check for |
btw, I got an error using some neat >v1.6 syntax: (; distribution, constraint, name) = distributions
ERROR: LoadError: LoadError: syntax: invalid assignment location "; distribution, constraint, name" around /home/runner/work/EnsembleKalmanProcesses.jl/EnsembleKalmanProcesses.jl/src/ParameterDistributions.jl:435
[...] so I changed it to be compatible... but maybe we should think about updating the version of our runners? Or are we explicitly wanting to support Julia 1.6 still? |
Looks Great!
|
With your suggestion, the following is now outputted: julia> c2 = [bounded(10, 15), Constraint{Affine}(transform, jac_transform, inverse_transform, Dict("linear" => 3, "const" => 14))]
2-element Vector{Constraint}:
Constraint{Bounded} with bounds (10, 15)
Constraint{Affine} with characterization ("linear" => 3, "const" => 14) |
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.
LGTM!
We can ignore codecov as it is not a sensible, so squash and merge, thanks!
* types: `Constraint`, `ParameterDistribution`
bors r+ |
Build succeeded: |
See #257 for details.
With this implementation, combining all the examples here gives a vector that displays like this:
Individual constraints render like this: