[BUG] Binary AMBER rst files need a "title" attribute

[lohedges]

As discovered in Exscientia/biosimspace#22, the binary AMBER restart file requires a title attribute to be used with pmemd for certain simulation types, specifically FEP. (No idea why this is the case.) From testing here all that seems to be required is that a title is present, i.e. it doesn't need to be anything specific.

[lohedges]

Looking at the code it looks like the title is read. It is also created from the system and appears to be set in the globals section along with the other information that I'm seeing in the file, e.g. applicatiion, program, programVersion, etc? Will try to figure out why it's not appearing. Perhaps the title, which is generated from the system name, is empty, so isn't being written?

[lohedges]

Yes, it's because the system name is empty, so no title is written. This patch fixes things. (Feel free to add whatever title you think suitable.) I'll backport to 2023.2.3 so Exs can move forward.

diff --git a/corelib/src/libs/SireIO/amberrst.cpp b/corelib/src/libs/SireIO/amberrst.cpp
index 04f7b93e..39cc87f4 100644
--- a/corelib/src/libs/SireIO/amberrst.cpp
+++ b/corelib/src/libs/SireIO/amberrst.cpp
@@ -293,6 +293,10 @@ void AmberRstFile::writeHeader(const QString &title, int nframes, int natoms,
     {
         globals.insert("title", title);
     }
+    else
+    {
+        globals.insert("title", "sire restart file");
+    }

     // now create the descriptions of all of the dimensions
     QHash<QString, NetCDFDataInfo> dimensions;

[chryswoods]

Thanks - yes, that's the right solution. I think sire restart file is a good enough title :-)

I'll check and test this locally, and will then push it into main and devel without CI/CD as it should all work, and I want to save the GH Actions. It will be ready for a 2023.3.2 release, if we make one, and will definitely be in 2023.4.0.

[chryswoods]

Ok - this is now in main and devel (above two commits). It is a slightly different fix because of the different way the new AmberRst parser constructs the title.

[chryswoods]

also, next issue is #100 :-)

[lohedges]

I'm guessing this happened for FEP due to the way that the system was created in BioSimSpace. We do set a default name, but perhaps the route bypassed this somehow. (This happens on conversion from any object to a system.)