Discuss changing options_.hp_ngrid

The option options_.hp_ngrid is now badly named after introducing a bandpass_filter option. It actually governs the number of points used in the inverse Fourier transform for any filter (see #1011 (closed)). I would suggest creating a new option ifft_points in the options structure and the preprocessor. For backward compatibility we should keep hp_ngrid in the preprocesor, but have it now map into options_.ifft_points

I think we also use inverse Fast Fourrier Transform for the estimation of the posterior densities, and probably elsewhere. So I wonder if a unique ifft_pointsoption is a good thing. With your suggestion, if I do not misunderstand, hp_ngrid would change ifft_points, ie the option considered for the bandpass_filter. This kind of side effect cannot be avoided if we use global options (global in the sense that the same option is used for different purposes). This design problem is very common in Dynare, so we should commit no to add more global options. I would suggest instead to add a new option (for instance bk_ngrid).

If we agree on this I can do this change, and reorganize the options in subfields in options_ (filter, filer.one_sided_hp, filter.two_sided, filter.bandpass_filter, filter.two_sided_filer.ifft_points, ...).

The number of ifft points for computing the posterior densities is separetely stored in kernel_options.gridpoints. The option I am talking about is hp_ngrid used in th_autocovariances.m, which governs the number of points used to approximate the theoretical moments of the model. Because we considered only hp-filtered moments initially, we named that option hp_ngrid. But with the introduction of bandpass-filtering, that name is probably misleading. Not accounting for backward compatibility, I would get rid of that option and rename it to something like filtered_theoretical_moments_grid. I don't see a reason to make this option filter-dependent. My suggestion to let hp_ngrid change that option was only for reasons of backward compatibility with older mod-files that set hp_ngrid

changed milestone to %4.6

I would still like to propose to add this to our list of changes for 4.6.

@sebastien I don’t like postponing this. It’s a decision we have been trying to make for four years. If we go for it, we should do it now. If not, that is also fine. Then we should close the issue.

changed milestone to %4.6

Ok, fair enough.

@JohannesPfeifer Just to be sure that I understand correctly, your proposal is to:

• introduce a new filtered_theoretical_moments_grid option, that sets the options_.filtered_theoretical_moments_grid field
• use this new field (instead of options_.hp_ngrid) when computing theoretical autocovariances and spectral density of variables
• keep the hp_ngrid option for backward compatibility, but have it set the options_.filtered_theoretical_moments_grid field instead

If this is what you mean, then that looks ok to me.

Yes, that is what I meant

@stepan-a Is this ok with you?

assigned to @sebastien

mentioned in commit preprocessor@1c44baea

closed via commit e371b1a9

mentioned in commit preprocessor@045e81d0

mentioned in commit 5daf56ef