Autosave support

[jsouter]

Addressing https://github.com/dls-controls/pythonSoftIOC/issues/162 Adding in support for EPICS autosave style periodic saving of PV values and fields to a yaml-based backup file, from which initial values can be loaded on a restart.

[jsouter]

Just tested with all the builder record types, looks like it's not happy with waveform record types (numpy array not serialisable to json), will either have to cast these to lists or try dumping to a yaml file, possibly with custom representers. Or use some sort of pickle-type approach, but probably preferable to be human readable.

[jsouter]

I have added support for autosaving non-VAL fields, which is done by calling builder.aOut("PV-NAME", autosave_fields=["SCAN", "PREC"]) for example. The autosave=True argument is used to determine if we add the VAL field to autosave, which I think is less ugly than requring the use to pass ["VAL"] to autosave_fields. (I note that this all needs to be added to the documentation when it's decided). I have noted however that In type records raise an exception when calling set_field, which gets called when loading from an autosave file. It could be possible to get around this by changing the setter to use the . accessor for fields, (pv.SCAN = "Passive") when loading, but we would have to make sure this load gets done before LoadDatabase() is called, which would be hard to enforce unless require the user to explicitly call something like a classmethod Autosave.load() before LoadDatabase() and softioc.iocInit().

EDIT: Have realised that alternatively we could force DISP=0 for In type records that require fields to be tracked in autosave.

[jsouter]

I will spend some time this afternoon looking back over original autosave and see if there's any features of that worth implementing. I'm not sure if there's any meaningful reason to try and implement things like the asVerify and the pass0/pass1 restoring of values before and after record initialization, but there are a few things like manual save restore that could be more relevant. I wonder if using the name autosave implies a sort of feature parity that we don't necessarily want

[AlexanderWells-diamond]

One last feature request: It may well be useful to provide a context manager, that can automatically do Autosave for all PVs declared within it. Possibly also supporting a specific list of fields, something like this:

with Autosave(autosave=True, autosave_fields=["EGU", "PREC"]):
  builder.aOut("MY_RECORD")

And this will autosave MY_RECORD's VAL, EGU, and PREC fields.

[jsouter]

One last feature request: It may well be useful to provide a context manager, that can automatically do Autosave for all PVs declared within it. Possibly also supporting a specific list of fields, something like this:

with Autosave(autosave=True, autosave_fields=["EGU", "PREC"]):
  builder.aOut("MY_RECORD")

And this will autosave MY_RECORD's VAL, EGU, and PREC fields.

I'll push fixes to the above comments soon, I've got lots of things in branches at the moment! The most straightforward way I could think to do this was to define a context manager in builder.py like this

_user_defaults = {}  # for use in DeviceFields context manager

class DeviceFields:
    def __init__(self, **fields):
        self._fields = fields

    def __enter__(self):
        global _user_defaults
        _user_defaults = self._fields

    def __exit__(self, A, B, C):
        global _user_defaults
        _user_defaults = {}

def _set_user_defaults(fields):
    fields.update(_user_defaults)

and have aOut etc call _set_user_defaults(fields).

This is generic enough that it could be used for any EPICS field like EGU etc, would that be something we want? Or would it be better to rework the Autosave interface to work as a context manager?

with builder.DeviceFields(autosave=True, EGU="mm"):
    builder.aOut("MY-RECORD")

[AlexanderWells-diamond]

I would prefer to see the Autosave class used as the context manager. As we don't have to run any code during the creation of each PV, what we can do is track how many PVs were created during the lifetime of the context manager and then add them in bulk to the Autosave list. Something like this (untested) code:

from softioc.device_core import LookupRecordList

class Autosave:
    def __init__(self, **fields):
        self._fields = fields

    def __enter__(self):
       self.before = LookupRecordList()

    def __exit__(self, A, B, C):
        after = LookupRecordList()
        # Get diff of the keys in before and after dict, then add them to Autosave

[jsouter]

I would prefer to see the Autosave class used as the context manager. As we don't have to run any code during the creation of each PV, what we can do is track how many PVs were created during the lifetime of the context manager and then add them in bulk to the Autosave list. Something like this (untested) code:

from softioc.device_core import LookupRecordList

class Autosave:
    def __init__(self, **fields):
        self._fields = fields

    def __enter__(self):
       self.before = LookupRecordList()

    def __exit__(self, A, B, C):
        after = LookupRecordList()
        # Get diff of the keys in before and after dict, then add them to Autosave

Got it, I think this specific implementation would be problematic because of circular imports, but I can see if I can come up with something that achieves the same thing. Of course could maybe avoid the import problem by importing within a class/function scope but could be a bit of a code smell. edit: never mind, looks like that's fine

[Araneidae]

this specific implementation would be problematic because of circular imports

The simplest fix for that would be to replace

from softioc.device_core import LookupRecordList

with

from softioc import device_core

and invoke as device_core.LookupRecordList: Python handles circular imports by postponing populating a recursively imported module until the imports are all done, so you can import a module recursively so long as you don't look inside it too early! (Maybe you already know all this...)

[jsouter]

I realise I was mistaken about how the device name work, I was under the impression that the device name gets preprended to PVs that were instantiated before builder.SetDeviceName() when builder.LoadDatabase() gets called. Since this is apparently not the case, I will undo the change to strip the device prefix from the autosave key. This will make it much simpler to support the edge case where users change the device name in the middle of the program and define multiple records with the same name (which I have seen in a real softioc in use at Diamond).

Considering the idea of multiple devices within an ioc, would we want to support the ability to backup differnet groups of PVs to multiple files, each with a different save period? I have drafted this with both sync and async methods, and it is a feature provided by C++ autosave.

[AlexanderWells-diamond]

I'm inclined to keep it simple, at least for the first release. If anyone wants the extra feature they can raise an issue for it.

[jsouter]

I believe I have managed to make the context managers thread safe by using a singleton style class that inherits from threading.local. I have moved some of the context manager logic into a new _AutosaveContext class, but if this seems a bit too convoluted see the approach in the commit https://github.com/DiamondLightSource/pythonSoftIOC/pull/163/commits/2da2fbf823d5651500164954effafe6c287d1e64 which applies all the singleton/threading logic to Autosave. Edit: also noting here that the tests are all passing on my machine, I will make sure to fix the build but have mostly been focusing on the actual code recently

[jsouter]

If we have a non-default list of autosave_fields for a PV inside an Autosave context manager, do we want to combine the lists or give priority to just the specified fields in the PV init? I suppose we still need to discuss if we want to track VAL inside autosave_fields instead of using autosave as its own kwarg.

[AlexanderWells-diamond]

I'd be inclined to merge the list of extra fields. If users want to have an override they can just remove that one PV from the context manager.

Regarding tracking VAL, it's an awkward one due to the assumption that a record name without .VAL is actually referring to the VAL field. If that's also how C++ Autosave works I'm inclined to leave it as it is - having autosave=True is what I expect most users will want, and is shorter than writing autosave_fields=["VAL"].

[Araneidae]

Regarding tracking VAL, it's an awkward one due to the assumption that a record name without .VAL is actually referring to the VAL field. If that's also how C++ Autosave works I'm inclined to leave it as it is - having autosave=True is what I expect most users will want, and is shorter than writing autosave_fields=["VAL"].

A hacky workaround that occurs to me is this:

• Get rid of autosave_fields
• Make autosave behave mostly like autosave_fields does now but...

• Allow autosave to be set to one of three types of value with the following interpretations:

  • A list (of strings) means autosafe all the listed fields, as for autosave_fields now
  • A value of True means the same as ['VAL']
  • A single string s means the same as [s] (this one may be a hack too far)

[AlexanderWells-diamond]

I think I'm ok with @Araneidae 's suggestion of using a single variable as either a bool, a list, or none. It keeps the shorthand for enabling saving for VAL, while also allowing flexibility for more fields. Only potential downside is that you would have to specify VAL in the list e.g. autosave=["VAL", "EGU"] - a minor concern but one that would be worth ensuring is highlighted in the docs.

[OCopping]

The builds are failing due to actions/upload-artifact@v2 being deprecated (see: here) To fix this, bump to either v3 or v4. However if bumping to v4, you will need to make the artifact name unique for each build, for example in https://github.com/epics-base/epicscorelibs/pull/29

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 93.96985% with 12 lines in your changes missing coverage. Please review.

Project coverage is 88.44%. Comparing base (413931e) to head (4248754). Report is 5 commits behind head on master.

Files with missing lines	Patch %	Lines
softioc/autosave.py	93.68%	12 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #163 +/- ## ========================================== + Coverage 87.33% 88.44% +1.11% ========================================== Files 14 15 +1 Lines 1066 1264 +198 ========================================== + Hits 931 1118 +187 - Misses 135 146 +11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[jsouter]

Regarding tracking VAL, it's an awkward one due to the assumption that a record name without .VAL is actually referring to the VAL field. If that's also how C++ Autosave works I'm inclined to leave it as it is - having autosave=True is what I expect most users will want, and is shorter than writing autosave_fields=["VAL"].

A hacky workaround that occurs to me is this:

• Get rid of autosave_fields
• Make autosave behave mostly like autosave_fields does now but...

• Allow autosave to be set to one of three types of value with the following interpretations:

  • A list (of strings) means autosafe all the listed fields, as for autosave_fields now
  • A value of True means the same as ['VAL']
  • A single string s means the same as [s] (this one may be a hack too far)

Have implemented all these, can remove support for passing a single string if we think it's too hacky.

[jsouter]

Great! I don't seem to have permissions to merge but happy to have it squashed with a message like "add support for saving and restoring pv fields with autosave"