search

kidusasfaw / spatPomp

R package for statistical inference of spatiotemporal partially observed Markov processes
GNU General Public License v3.0
11 stars 10 forks source link

Fixed simulate bug and created argument to save hidden state in bpfilter. #10

Closed jeswheel closed 2 years ago

jeswheel commented 2 years ago

In the simulate function, there was a bug when using the option format = 'data.frame'. The bug occurs when the unit-state names are nested, e.g. defining a state "VR1D" and "VR1DD". This is because the code would search for a state type for each unit, and here it would think that the state type for "VR1DD10" (the state for the 10th unit) is "VR1D" instead of "VR1DD", causing it to say the unit number is "D10", rather than "10". This bug was fixed by just using the number at the end of each statename. This of course introduces a potential bug that if a unit has a state name that ends in a number, it will not be able to accurately extract the unit number, although I think that such statenames result other bugs within the function, and therefore this naming scheme should be avoided.

The other change involves a save_state argument in the bpfilter function. For the project I'm working on, I need access to the hidden states; this argument is a feature of pomp::pfilter, but not spatPomp::bpfilter. I just made a few minor adjustments so that the bpfilter function will now accept a save_state argument, and I implemented it in a way so as to mimic the existing implementation in the pomp package.

kingaa commented 2 years ago

Might it be a good idea, in view of this kind of issue, to use some punctuation to separate variable name and unit name? See for example the tidyverse functions unite and separate.

ionides commented 2 years ago

@kingaa, we considered that. panelPomp appends "[u]" for the name of the unit-specific parameters. For ease of working with C snippets, it may be better to use "_u". Either way, this seems like a nontrivial change to spatPomp, so I'll go ahead and merge @jeswheel's edits for now.