XZInterpolation rename, and passing Options to XZInterpolation objects

[johnomotani]

Rename Interpolation to XZInterpolation as it only supports a specific type of interpolation, and we might introduce other types in future.

Update the XZInterpolation constructor so it accepts an Options* opt argument, so the options can be used by the implementations.

Structure the sections for the options controlling ParallelTransform objects and XZInterpolation objects more sensibly. Previously: all Interpolation objects used the [interpolation] section; FCITransform objects used an [fci] section; and the type of parallel transform was set by mesh:paralleltransform. Now the ParallelTransforms are created using a paralleltransform subsection of the mesh options ([mesh:paralleltransform]) by default, with the type set by e.g.

[mesh:paralleltransform]
type = fci

The interpolations used by FCI take their type (and any other options that might be added in future) from a subsection of the paralleltransform section, e.g.

[mesh:paralleltransform:xzinterpolation]
type = lagrange4pt

Introduces another convenience overload for the generic Factory, Factory::create(Options* options, Args&&... args) , that makes using factories inside another class neater, when following the pattern: calling class gets passed Options section in its constructor; calling class passes relevant subsection to member objects (and the subsection should determine the particular type of a generic member object).

Breaking changes:

• Interpolation class is renamed to XZInterpolation - should not affect user code
  • All the implementations are also renamed XZHermiteSpline, etc.
• XZInterpolation declaration moved from include/interpolation.hxx to include/interpolation_xz.hxx - should not affect user code
  • note most uses of #include "interpolation.hxx" in user code are probably for interp_to(), which is not affected by this PR, and not for XZInterpolation`
• Type of parallel transform is now set in type entry of [mesh:paralleltransform] section, instead of paralleltransform entry of [mesh] section
• FCI options are set in [mesh:paralleltransform] instead of [fci]
  • The only option at the moment was fci:z_periodic, which is now mesh:paralleltransform:z_periodic
• Default section for XZInterpolation options is [xzinterpolation] instead of [interpolation]
• Interpolation used by a ParallelTransform uses options from a subsection of [mesh:paralleltransform], for example the FCI interpolation options are set in [mesh:paralleltransform:xzinterpolation]

[dschwoerer]

Rename Interpolation to XZInterpolation as it only supports a specific type of interpolation, and we might introduce other types in future.

I am not sure we should make it more complicated then needed. Are you intending to introduce other interpolation schemes?

Structure the sections for the options controlling ParallelTransform objects and XZInterpolation objects more sensibly. Previously: all Interpolation objects used the [interpolation] section; FCITransform objects used an [fci] section; and the type of parallel transform was set by mesh:paralleltransform. Now the ParallelTransforms are created using a paralleltransform subsection of the mesh options ([mesh:paralleltransform]) by default, with the type set by e.g.

Could you provide a script to fix input files automatically?

[johnomotani]

Rename Interpolation to XZInterpolation as it only supports a specific type of interpolation, and we might introduce other types in future.

I am not sure we should make it more complicated then needed. Are you intending to introduce other interpolation schemes?

Yes, there's a ZInterpolation coming in #1803.

Could you provide a script to fix input files automatically?

@ZedThree suggested on Slack that we do so https://bout-project.slack.com/archives/CJYH9Q70X/p1579779782011800. It should be possible, but I'm not planning to write one due to lack of time. I think a good start would be to make an input file parser that preserves structure and comments when it writes out again (which the BoutOptionsFile Python class does not do). Could be a feature to add to boutproject/xBOUT#94?

[johnomotani]

a good start would be to make an input file parser that preserves structure and comments when it writes out again (which the BoutOptionsFile Python class does not do).

If that was done, we could use something like this

#!/usr/bin/env python3

from boutdata.data import BoutOptionsFile
from copy import deepcopy

def fixInputFile4to5(f):
    # fix parallel transform options, see PR #1896
    if 'mesh' in f:
        meshopts = f.getSection('mesh')
        if 'paralleltransform' in meshopts:
            ptvalue = meshopts['paralleltransform']
            del meshopts['paralleltransform']
            ptopts = meshopts.getSection('paralleltransform')
            ptopts['type'] = ptvalue
    if 'fci' in f:
        if f['fci'].sections():
            raise ValueError('Do not know what to do with subsections of [fci]')
        ptopts = f.getSection('mesh').getSection('paralleltransform')
        for key in f['fci'].keys():
            ptopts[key] = f['fci'][key]
        del f['fci']

    # fix interpolation options, see PR #1896
    if 'interpolation' in f:
        if f['interpolation'].sections():
            raise ValueError('Do not know what to do with subsections of [interpolation]')
        newsection = meshopts.getSection('paralleltransform').getSection('xzinterpolation')
        for key in f['interpolation'].keys():
            newsection[key] = f['interpolation'][key]
        del f['interpolation']

    return f


if __name__ == '__main__':
    from sys import argv, exit

    filename = argv[1]
    
    f = BoutOptionsFile(filename)
    
    f = fixInputFile4to5(f)
    
    while True:
        user_input = input('Do you want to overwrite "'+filename+'"? [y/N/v/h]').lower()
        
        if user_input == 'y':
            f.write(filename=filename, overwrite=True)
            exit(0)
        elif user_input == 'n' or user_input == '':
            exit(0)
        elif user_input == 'v':
            print(f)
        elif user_input == 'h':
            print('y - overwrite "'+filename+'" with the fixed version\n'
                  + 'n (default) - exit without changing "'+filename+'"\n'
                  + 'v - view the fixed input file\n'
                  + 'h - print this help')

[Note: to run the above example needs a method adding to BoutOptions

    def __delitem__(self, key):
        """
        Delete a key
        """
        key = key.lower()
        if key in self._keys:
            del self._keys[key]
        elif key in self._sections:
            del self._sections[key]
        else:
            raise KeyError(key)

and with the current BoutOptionsFile it clobbers comments, whitespace, and possibly the order of sections/options.]

[ZedThree]

LGTM. I'll add updater scripts next week

[ZedThree]

Turns out tests/integrated/test-interpolate/runtest sets interpolation:type=<type> on the command line, so that also needs updating to xzinterpolation:type, and tests/MMS/spatial/fci/runtest sets fci:y_periodic -> mesh:paralleltransform:y_periodic.

Updating command line options in scripts might be trickier to do both automatically and accurately.

Also, I think I missed why adding Options to XZInterpolation is necessary. I can't see where it's used?

[johnomotani]

Also, I think I missed why adding Options to XZInterpolation is necessary. I can't see where it's used?

That does seem to be true. I suspect that I just thought that given we have an xzinterpolation section, it would be nice to keep the options in the XZInterpolation objects, just in case any of the implementations ever needed some settings. I think at the moment xzinterpolation:type is the only setting, and that is only needed by the factory not the XZInterpolation classes themselves. Would you rather remove the Options member until it's needed?

[ZedThree]

Would you rather remove the Options member until it's needed?

I think that would be better. I found it a bit confusing, as I thought I was missing something.