Skip to content

New MCStep object #244

Closed
Closed

Description

@dwhswenson

@jhprinz and I discussed the fact that analysis is a bit confusing because some things are studied using the SampleSet and some are studied using the PathMoveChange. Furthermore, it sounds like some of his changes to speed up loading of PathMoveChanges may make it harder to loop over them in a one-per-MC-step way.

We realized that we can solve all of this by saving an MCStep object which has attributes for both the newly-determined SampleSet and the most recent PathMoveChange (from the top of the tree). This should make analysis a good bit easier from the user's point of view.

Activity

  1. dwhswenson commented on May 13, 2015

    @dwhswenson
    MemberAuthor

    Related to this: if we have such an object, it might be possible to create something like slices for storage based on subsets of this. A couple examples where I would want something like that:

    • while developing analysis methods, it can be nice to work on a small subset of the data for speed purposes
    • block averaging can be easily implemented by using slices

    If each step contains both the SampleSet and PathMoveChange, slicing the storage would just be slicing the steps list.

  2. jhprinz commented on May 15, 2015

    @jhprinz
    Contributor

    Will be part of #251.

  3. self-assigned this
    on May 15, 2015
  4. jhprinz commented on May 17, 2015

    @jhprinz
    Contributor

    Alright. I have this implemented in #251. The current MCStep has the following attributes

    class MCStep(OPSObject):
        """
        A monte-carlo step in the main PathSimulation loop
    
        It references all objects created and used in a MC step: 
        The used mover, and simulator as well as the initial and final sampleset, 
        the step number and the generated pathmovechange.
    
        Attributes
        ----------
        simulation : PathSimulation
            the running pathsimulation responsible for generating the step
        step : int
            the step number counting from the root sampleset
        pre : SampleSet
            the initial (pre) sampleset
        post : SampleSet
            the final (post) sampleset
        change : PathMoveChange
            the pathmovechange describing the transition from pre to post
        root : SampleSet
            the first sampleset at step 0.
        """
    

    Questions:

    • Is the naming good?
    • Do we need both .pre and .post? I thought this makes it very clear that this is a step which goes from a pre-state to a post-state and not a state itself.
    • Do we need the root? This is somehow redundant and in most cases the same as steps[0].pre
    • Do we want an additional reference to the used mover? This is accessible by step.change.mover. We could add this as a property. Actually I think I just do that.

    @dwhswenson suggested to allow to splice a store. Do you mean just internally and do something like:

    splice_store = storage.splice_steps(slice(0,None,10))
    

    which would just generate a fake storage that only contains every 10th step. Or really create a new storage as a reduced copy that only contains what is needed.
    Third option is to just add a slicing in the analysis to not use all data.

  5. dwhswenson commented on May 17, 2015

    @dwhswenson
    MemberAuthor

    Is the naming good?

    We might make things a bit more explicit.

    step to step_number (or even just number) -- otherwise, common usage might look weird:

    for step in steps:
        step_number = step.step

    (or worse yet, a user might be tempted to make that line step = step.step, and then spend a while wondering why the analysis crashes so hard)

    I would also suggest a different naming on pre and post. You've gone with a very symmetric treatment, but I don't think people will think of them symmetrically. Usually, you just want the new one (post). Keeping the previous around mainly serves as a sanity check, right? And a way to store the "-1" (root) step?

    I'd change post to current or active (possibly followed by _state or _samples), and pre to previous (with the same suffix, if you add one).

    change, simulation, and root all make sense to me (although if you suffix the above, root should also take that suffix).

    Do we need both .pre and .post?

    There are two reasonable ways to think about what the record of a simulation (which the list of MCSteps is) fundamentally represents:

    • it is a series of $N$ states, with metadata about how you got to state $n$ from state $n-1$
    • it is a series of $N-1$ changes, with convenience methods to obtain the state at either end

    They are, of course, equivalent; the question is which should our code reflect. You seem to be going toward the latter. I think that is the less intuitive approach and will seem unfamiliar to novices, so we should make things look more like the former.

    Keeping pre and post, but with less symmetric names, is, I think, a decent compromise of the two.

    Do we need the root?

    I don't know. The answer to that question should be the same as the answer to this one: Is there ever a reason within the analysis of a specific step that we would need to know what the root is?

    I can't think of a case for that, but that doesn't mean it doesn't exist.

    Note also that if you're doing block analysis, you basically split your steps into "independent" simulations. In that analysis context, the root for block 5 should have changed from what it was in the global analysis. So this makes me lean again keeping the root property.

    Do we want an additional reference to the used mover?

    I like the property idea. I assume you mean here that you'll return the canonical_mover from #243?

    Splicing storage

    I'm not entirely sure what implementation I'd like to see here, but let me explain the features. Also, some of the implementation changes might (probably will) need to happen in the analysis functions.

    This is all about making analysis functions reusable. I can immediately think of 3 contexts where it might be nice to use the same analysis function on different data:

    1. global analysis on all of the data in storage
    2. subset analysis on some group of steps, e.g., as would be used to obtain error estimates by block averaging or resampling
    3. per-replica analysis, as with Gelman-Rubin diagnostics

    Originally, I wanted to have the analysis functions take storage as their argument, because that way I could include storage.*.cache_all() in the analysis function and therefore not require the user to call it manually. Then, a sufficiently nice slicing behavior that makes the sliced storage look like a normal storage, but providing a different list of steps, would be a simple way to solve subset analysis.

    But I still don't see a simple solution to per-replica analysis. This requires some sort of filtering of the SampleSets in the step.post.

    Perhaps the best approach is to change the analysis functions to take a list/generator of MCSteps, and to apply some sort of extra step to filter the selected replica ID for the per-replica analysis. This also has the downside that storage.*.cache_all() needs to go into functions that the user must call. But we can combine all of them in storage.run_this_first_for_faster_analysis() (well, maybe not that name...)

  6. dwhswenson commented on May 17, 2015

    @dwhswenson
    MemberAuthor

    Actually, regarding storage.*.cache_all(): is there any reason not to make that a part of the loading of the storage file? At the end of storage.__init__ just do

    if mode=='r': 
        storage.changes.cache_all()
        storage.samples.cache_all()
        storage.samplesets.cache_all()
        storage.mcsteps.cache_all()

    Maybe add another flag in initialization like cache_on_load=True and check that in the if statement as well? You could also create a different mode, like "r-nocache" which you catch before sending mode "r" to netCDF. But I think caching by default when opening read-only will be the most user-friendly behavior.

    You might also, at each cache step, output something (at least to log, possibly to screen) saying "Loading N changes", "Loading N samples", etc, just so the user knows that the code is running correctly and why it is taking a couple seconds.

    In that case, the analysis functions should work on a list of MCSteps instead of Storage objects, and this removes the need for slicing storage: subset analysis works by explicitly creating a subset of storage.mcsteps, and per-replica analysis requires creating a filter that takes a list of MCSteps and returns a list of MCSteps with a different (filtered) SampleSet; that new list is then passed into the analysis routines.

    I think this solves all my concerns. So, to summarize:

    • don't worry about slicing storage
    • cache_all() stuff should be done by default as part of loading the file for reading (with an option to not do that if the user desires)
    • analysis routines should all be based on lists of MCSteps instead of storage. This may break a few things around automatically checking whether we're using the same storage as previously; I think I want to readjust that paradigm though (if, for some reason, you're analyzing the same transition object using a different storage, after analyzing it with a previous storage, then you're doing something advanced enough that you should know how to use the force=True parameter)
  7. jhprinz commented on May 17, 2015

    @jhprinz
    Contributor

    Few comments:

    • it is a series of $N$ states, with metadata about how you got to state $n$ from state $n-1$
    • it is a series of $N-1$ changes, with convenience methods to obtain the state at either end

    Yes, that was my question. The name step seemed to hint to the latter, that is why I used it and had a function to get the previous state and final state. If you say active or current is the post, then this solves another question on which we should use. It also means that if we want to use all states we have to add the initial one extra or we add a step that goes from nothing to the first state.
    We should add that to the pathSimulation as the initial step.

    The root would be useful as a reference . Since a simulation does the same mover several times we would have

    current_state = root_state + mover * (step_number + 1)
    

    Not sure if that is necessary, but it could be nice to have this additional information.
    The additional initial mcstep makes this a little redundant since we can just go back step_number steps to get the initial one. Maybe I remove this for now. We can add it again, if we really have a use for it.

    The .mover property would just return mcstep.change.mover and would this return the root_mover of the simulation. Canonical needs to be applied in addition mcstep.change.mover.canonical

    Auto-Caching

    Good idea. I had that in the code at some point and removed it because of memory concerns. I think it should be default to cache these with an option to turn it off.

    I will also add fast load/save/cache for mcstep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions