Metropolis Algorithm for MCMC #1262

wangcj05 · 2020-07-06T22:34:02Z

Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

close #1260
This PR will also update the submodule ID of LOGOS. Recent changes in RAVEN dependencies file structure caused the failure of LOGOS regression test. This has been recently fixed in LOGOS. see issue #1114
In addition, this PR also make some improvement on the restart capability (see #1280), this is because metadata is required for several Samplers when restarted is requested.

Implementation of Markov Chain Monte Carlo algorithm, i.e. Metropolis Algorithm.

What are the significant changes in functionality due to this change request?

For Change Control Board: Change Request Review

The following review must be completed by an authorized member of the Change Control Board.

1. Review all computer code.
2. If any changes occur to the input syntax, there must be an accompanying change to the user manual and xsd schema. If the input syntax change deprecates existing input files, a conversion script needs to be added (see Conversion Scripts).
3. Make sure the Python code and commenting standards are respected (camelBack, etc.) - See on the wiki for details.
4. Automated Tests should pass, including run_tests, pylint, manual building and xsd tests. If there are changes to Simulation.py or JobHandler.py the qsub tests must pass.
5. If significant functionality is added, there must be tests added to check this. Tests should cover all possible options. Multiple short tests are preferred over one large test. If new development on the internal JobHandler parallel system is performed, a cluster test must be added setting, in XML block, the node <internalParallel> to True.
6. If the change modifies or adds a requirement or a requirement based test case, the Change Control Board's Chair or designee also needs to approve the change. The requirements and the requirements test shall be in sync.
7. The merge request must reference an issue. If the issue is closed, the issue close checklist shall be done.
8. If an analytic test is changed/added is the the analytic documentation updated/added?
9. If any test used as a basis for documentation examples (currently found in raven/tests/framework/user_guide and raven/docs/workshop) have been changed, the associated documentation must be reviewed and assured the text matches the example.

moosebuild · 2020-07-06T23:01:17Z

Job Test CentOS 7 on 8f116ea : invalidated by @wangcj05

conda error when setting python environment

alfoa · 2020-07-08T20:08:09Z

framework/Samplers/MCMC/MCMC.py

+
+  def localInputAndChecks(self, xmlNode, paramInput):
+    """
+      unfortunately-named method that serves as a pass-through for input reading.


unfortunately-named method? O_o

10:1 says I wrote the original version this was copied from ... heh

PaulTalbot-INL · 2020-07-08T20:41:42Z

framework/Samplers/MCMC/MCMC.py

+      else:
+        self._proposal[var] = None
+    # TargetEvaluation Node (Required)
+    targetEval = paramInput.findFirst('TargetEvaluation')


I believe the AdaptiveSampler does this for you, so it should not be needed here if you call the base class, right?

Currently, the adaptive sampler using following lines to define and retrieve TargetEvaluation:

self.addAssemblerObject('TargetEvaluation', 'n') ... self._targetEvaluation = self.assemblerDict['TargetEvaluation'][0][3]

Which in my opinion should be changed to use

self.retrieveObjectFromAssemblerDict

alfoa · 2020-07-10T20:43:33Z

@wangcj05 I like the generality of allowing the user to use any distribution for the proposal (following the general metropolis-hasting algo), but I am afraid that we will "pay" this generality in terms of convergence issues and/or unexpected behavior. For example, how do we handle the discrete distributions here? Multivariates?

alfoa · 2020-07-10T22:49:28Z

@wangcj05 as during our conversation, if we limit the usability of the proposal distributions and check the truncability, I am good for the merge

mandd · 2020-07-10T23:16:04Z

framework/Distributions.py

@@ -439,6 +439,15 @@ def pdf(self,x):
    returnPdf = self._distribution.pdf(x)
    return returnPdf

+  def logpdf(self,x):


I am having my revenge here: should it be logPdf?

mandd · 2020-07-10T23:19:03Z

framework/Samplers/MCMC/MCMC.py

+    """
+    descr = r"""
+    \section{Markov Chain Monte Carlo} \label{sec:MCMC}
+    The Markov chain Monte Carlo (MCMC) is another important entity in the RAVEN framework.


... is another important sampler ...

mandd · 2020-07-10T23:21:15Z

framework/Samplers/MCMC/MCMC.py

+
+  def localFinalizeActualSampling(self, jobObject, model, myInput):
+    """
+      General function (available to all samplers) that finalize the sampling


mandd · 2020-07-10T23:22:23Z

framework/Samplers/MCMC/MCMC.py

+      @ In, externalSeeding, int, optional, external seed
+      @ In, solutionExport, DataObject, optional, a PointSet to hold the solution
+      @ Out, None
+    """


This is a generic question: does this MCMC method allow the user to use a chain generated in the past that needs to be augmented?

I think the answer for this question is No for current implementation. This seems to me similar to Restart option.

No restart?

Can you try a test for it?

mandd · 2020-07-10T23:24:27Z

I have completed a first round of comments; I need a bit more time to think more about it

wangcj05 · 2020-07-14T22:44:00Z

@alfoa I have added the checks for Distributions. Only continuous distribution is allowed.

wangcj05 · 2020-07-14T22:47:24Z

@mandd I have addressed your comments.

alfoa · 2020-07-15T13:06:16Z

framework/Samplers/MCMC/MCMC.py

+      @ In, externalSeeding, int, optional, external seed
+      @ In, solutionExport, DataObject, optional, a PointSet to hold the solution
+      @ Out, None
+    """


No restart?

alfoa · 2020-07-15T13:06:31Z

framework/Samplers/MCMC/MCMC.py

+      @ In, externalSeeding, int, optional, external seed
+      @ In, solutionExport, DataObject, optional, a PointSet to hold the solution
+      @ Out, None
+    """


Can you try a test for it?

alfoa · 2020-07-15T13:08:26Z

tests/framework/MCMC/test_metropolis.xml

+      mean: [5, 5], cov=[[1, 0.9], [0.9, 1]]
+      Both input parameters have the standard normal distribution as their prior distribution.
+      The proposal distributions for the input variables are also standard normal distribution.
+      "TargetEvaluation" is used to collect the inputs and outputs from the likelihood model.


since this goes into a latex file, I would use the `` '' construct for the quotes

wangcj05 · 2020-07-16T16:53:27Z

@alfoa I have opened an issue for the restart problem, see #1280. As we discussed, it is a basic issue for raven, and we need a discussion on this. I think a different PR need to be created to address the issue. Please let me know what you think.

moosebuild · 2020-07-20T14:58:12Z

All jobs on 87b7177 : invalidated by @alfoa

alfoa · 2020-07-23T21:47:02Z

@wangcj05 do you want to develop the restart here or in another PR?

wangcj05 · 2020-07-24T18:57:54Z

@wangcj05 do you want to develop the restart here or in another PR?

I have updated the Metropolis Sampler with the restart capability. The original test has also been updated with the restart.

wangcj05 · 2020-07-24T21:44:22Z

@alfoa I have addressed all your comments, restart is now tested, and the tests are green now.

alfoa · 2020-07-25T17:50:11Z

Checklist passed...

Comments have been addressed...

I approve this PR...

Merging...

@wangcj05 can you update the issue #1225 for the new release changelog?

wangcj05 added 21 commits June 25, 2020 17:31

add logpdf for Distributions

4c6900e

MCMC initial implementation

00f2cf7

update MCMC

e6c1253

update

1cb20bf

update test for MCMC

364053c

register MCMC to Simulation and Steps

10791ab

updates

a8999a5

first draft implementation

eb79308

restructure metropolis sampling

874eef8

enable test for mcmc metropolis algorithm

e83077d

clean up and update docstrings

73ac3da

update tests

7c4763c

rename test

4baa521

fix unit test for inputData

75a1378

Merge remote-tracking branch 'origin/devel' into wangc/mcmc_mh

a17ae22

remove MCMC actor in Steps

fb505af

move MCMC to Samplers

9875a29

update code structure

d1b9790

complete conversion from MCMC to Metropolis Sampler

a0d2813

add xsd for Metropolis

5d61ba4

update test for xsd schema

8f116ea

wangcj05 requested a review from alfoa July 6, 2020 22:34

wangcj05 added the RAVENv2.1 All tasks and defects that will go in RAVEN v2.1 label Jul 6, 2020

alfoa requested changes Jul 8, 2020

View reviewed changes

PaulTalbot-INL reviewed Jul 8, 2020

View reviewed changes

mandd reviewed Jul 10, 2020

View reviewed changes

wangcj05 added 6 commits July 14, 2020 10:43

address reviewers's comments

be5f93c

add user manual for MCMC

fc73e66

finalize the user manual for MCMC

8b95e74

change tune to burnIn

b179c21

enable burnIn for SolutionExport

cc3d861

add checks for distributions

cafceee

wangcj05 added 2 commits July 14, 2020 16:47

update user manual

e359266

fix extra getDistType in Grid

37740ce

alfoa requested changes Jul 15, 2020

View reviewed changes

update test description

87b7177

wangcj05 added 3 commits July 24, 2020 12:03

Merge branch 'devel' into wangc/mcmc

b5b2b68

add restart capability for Metropolis Sampler

81dd7c8

add gold csv for restart

3338b82

wangcj05 added 3 commits July 24, 2020 13:37

update user manual, accept log likelihood for MCMC

71dd052

update LOGOS submodule ID

e8d7af2

update xsd

8b43afe

alfoa mentioned this pull request Jul 25, 2020

[TASK] Markov Chain Monte Carlo: Metropolis Algorithm #1260

Closed

10 tasks

alfoa approved these changes Jul 25, 2020

View reviewed changes

alfoa merged commit 2a94807 into devel Jul 25, 2020

alfoa deleted the wangc/mcmc branch July 25, 2020 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metropolis Algorithm for MCMC #1262

Metropolis Algorithm for MCMC #1262

wangcj05 commented Jul 6, 2020 •

edited by alfoa

Loading

moosebuild commented Jul 6, 2020

alfoa Jul 8, 2020

PaulTalbot-INL Jul 8, 2020

wangcj05 Jul 14, 2020

PaulTalbot-INL Jul 8, 2020

wangcj05 Jul 14, 2020

alfoa commented Jul 10, 2020

alfoa commented Jul 10, 2020

mandd Jul 10, 2020

wangcj05 Jul 14, 2020

mandd Jul 10, 2020

wangcj05 Jul 14, 2020

mandd Jul 10, 2020

wangcj05 Jul 14, 2020

mandd Jul 10, 2020

wangcj05 Jul 14, 2020

alfoa Jul 15, 2020

alfoa Jul 15, 2020

mandd commented Jul 10, 2020

wangcj05 commented Jul 14, 2020

wangcj05 commented Jul 14, 2020

alfoa Jul 15, 2020

alfoa Jul 15, 2020

alfoa Jul 15, 2020

wangcj05 Jul 16, 2020

wangcj05 commented Jul 16, 2020

moosebuild commented Jul 20, 2020

alfoa commented Jul 23, 2020

wangcj05 commented Jul 24, 2020

wangcj05 commented Jul 24, 2020

alfoa commented Jul 25, 2020

Metropolis Algorithm for MCMC #1262

Metropolis Algorithm for MCMC #1262

Conversation

wangcj05 commented Jul 6, 2020 • edited by alfoa Loading

Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

What are the significant changes in functionality due to this change request?

For Change Control Board: Change Request Review

moosebuild commented Jul 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alfoa commented Jul 10, 2020

alfoa commented Jul 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mandd commented Jul 10, 2020

wangcj05 commented Jul 14, 2020

wangcj05 commented Jul 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangcj05 commented Jul 16, 2020

moosebuild commented Jul 20, 2020

alfoa commented Jul 23, 2020

wangcj05 commented Jul 24, 2020

wangcj05 commented Jul 24, 2020

alfoa commented Jul 25, 2020

wangcj05 commented Jul 6, 2020 •

edited by alfoa

Loading