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.

Sign up for GitHub

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

Jump to bottom

Instantiation of new AbstractMetrics #45

Closed
willtebbutt opened this issue Apr 2, 2019 · 3 comments
Closed

Instantiation of new AbstractMetrics #45

willtebbutt opened this issue Apr 2, 2019 · 3 comments
Labels
question
Milestone
Release v0.2

Comments

@willtebbutt
Copy link
Contributor

We currently have a method for each AbstractMetric which enables us to make another instance of the metric of the following form:

metric = ...
new_metric_of_same_type = metric(inverse_mass_matrix)

Why do we need such a method?
@willtebbutt willtebbutt changed the title Immutability of AbstractMetrics Instantiation of new AbstractMetrics Apr 2, 2019
@xukai92
Copy link
Member

xukai92 commented Apr 2, 2019

This is for adaptation, during which we create a new metric based on the new M inverse. Hong and I believe make metric immutable instead of change in place would make code easier to maintain.

@willtebbutt
Copy link
Contributor Author

I see -- I'm fully on board with the motivation for this, but I don't know that I think the current interface is ideal.

In particular, it's not clear to me why calling an object of type AbstractMetric should create another object of type AbstractMetric -- I would naively have expected the evaluation of a metric to take two elements from the corresponding space and compute the distance between them. Would you consider renaming this to something more informative?

I do appreciate that I'm being a bit pedantic...

@xukai92
Copy link
Member

xukai92 commented Apr 2, 2019

Yes I'm open to rename it.

@xukai92 xukai92 mentioned this issue Apr 3, 2019
Merged
@yebai yebai mentioned this issue May 17, 2019
Closed
10 tasks
@xukai92 xukai92 added this to the Release v0.2 milestone Jul 9, 2019
This was referenced Jul 10, 2019
Merged
Merged
xukai92 added a commit that referenced this issue Jul 11, 2019
@xukai92
implement renew for AbstractMetric; all caller renwers are removed (#45)
3120395
xukai92 added a commit that referenced this issue Jul 23, 2019
@xukai92
v0.2.0 (#81)
fe3de8b 
* introduce Divergence type

* change the filed name of divergence from s to div in FullBinaryTree

* draft the interface to return stats

* improve the formatting of verbose

* implement Base.show for user side types (#36)

* remove return_stats - need to check @cpfiffer's interface change

* RFC Adapter types #28

* change version number to v0.2.0

* add step size jittering

* quote Stan

* implement renew to all internal types except AbstractMetric

* remove unused caller renew

* implement renew for AbstractMetric; all caller renwers are removed (#45)

* improve _string_diag

* return stats via NamedTuple

* fix test and update README.md

* ignore logdensity and graident function in Base.show for Hamiltonian to avoid stackoverlfow in Turing

* remove unused stats type

* update named tuple signature

* Divergence -> Termination

* div -> termination

* remove renew except those for metric

* remove renew except those for metric

* remove renew except those for metric

* remove renew for metric
@xukai92 xukai92 closed this as completed Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
question
Projects
None yet
Milestone
Release v0.2
Development

No branches or pull requests
2 participants
@willtebbutt @xukai92