From 0e9d656ba2f900c488d1851c4f140860b88c29c6 Mon Sep 17 00:00:00 2001
From: David Keitel <david.keitel@ligo.org>
Date: Tue, 5 Nov 2019 16:54:55 +0000
Subject: [PATCH] Writer: clean up logic in check_cached_data_okay_to_use()

 -make clearer when and why we test the .cff file
 -also improve logging output in this function
  and in check_if_cff_file_needs_rewritting()
 -test_makefakedata_usecached: add a call to make_cff()
  in 2nd run as an extra check
---
 pyfstat/make_sfts.py | 70 ++++++++++++++++++++++++--------------------
 tests.py             |  4 +++
 2 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/pyfstat/make_sfts.py b/pyfstat/make_sfts.py
index 4a7dca9..6cdc245 100644
--- a/pyfstat/make_sfts.py
+++ b/pyfstat/make_sfts.py
@@ -259,46 +259,51 @@ transientTau = {:10.0f}\n"""
     def check_cached_data_okay_to_use(self, cl_mfd):
         """ Check if cached data exists and, if it does, if it can be used """
 
-        getmtime = os.path.getmtime
+        need_new = "Will create new SFT file."
 
+        logging.info("Checking if cached data good to reuse...")
         if os.path.isfile(self.sftfilepath) is False:
-            logging.info("No SFT file matching {} found".format(self.sftfilepath))
+            logging.info("No SFT file matching {} found. {}".format(self.sftfilepath, need_new))
             return False
         else:
-            logging.info("Matching SFT file found")
-
-        if getmtime(self.sftfilepath) < getmtime(self.config_file_name):
-            logging.info(
-                (
-                    "The config file {} has been modified since the sft file {} "
-                    + "was created"
-                ).format(self.config_file_name, self.sftfilepath)
-            )
-            return False
+            logging.info("OK: Matching SFT file found.")
+
+        if "injectionSources" in cl_mfd:
+            if os.path.isfile(self.config_file_name):
+                if os.path.getmtime(self.sftfilepath) < os.path.getmtime(self.config_file_name):
+                    logging.info(
+                        (
+                            "The config file {} has been modified since the SFT file {} "
+                            + "was created. {}"
+                        ).format(self.config_file_name, self.sftfilepath, need_new)
+                    )
+                    return False
+                else:
+                    logging.info(
+                        "OK: The config file {} is older than the SFT file {}".format(
+                            self.config_file_name, self.sftfilepath)
+                    # NOTE: at this point we assume it's safe to re-use, since
+                    # check_if_cff_file_needs_rewritting()
+                    # should have already been called before
+                    )
+            else:
+                raise RuntimeError("Commandline requires file '{}' but it is missing.".format(self.config_file_name))
 
-        logging.info(
-            "The config file {} is older than the sft file {}".format(
-                self.config_file_name, self.sftfilepath
-            )
-        )
-        #logging.info("Checking contents of cff file") # FIXME: check doesn't seem to exist...?
-        #if not XXX:
-            #logging.info("Contents unmatched")
-            #return False
-        logging.info("Checking commandline against existing SFT header")
+        logging.info("Checking new commandline against existing SFT header...")
         cl_dump = "lalapps_SFTdumpheader {} | head -n 20".format(self.sftfilepath)
         output = helper_functions.run_commandline(cl_dump)
         header_lines_lalapps = [line for line in output.split("\n") if "lalapps" in line]
         if len(header_lines_lalapps)==0:
-            logging.info("Could not obtain comparison commandline from old SFT header")
+            logging.info("Could not obtain comparison commandline from old SFT header. "+need_new)
             return False
         cl_old = header_lines_lalapps[0]
-        if helper_functions.match_commandlines(cl_old,cl_mfd):
-            logging.info("Commandline matched with old SFT header, use old sft file")
-            return True
-        else:
-            logging.info("Commandlines unmatched, create new sft file")
+        if not helper_functions.match_commandlines(cl_old,cl_mfd):
+            logging.info("Commandlines unmatched. "+need_new)
             return False
+        else:
+            logging.info("OK: Commandline matched with old SFT header.")
+        logging.info("Looks like cached data matches current options, will re-use it!")
+        return True
 
     def check_if_cff_file_needs_rewritting(self, content):
         """ Check if the .cff file has changed
@@ -306,24 +311,25 @@ transientTau = {:10.0f}\n"""
         Returns True if the file should be overwritten - where possible avoid
         overwriting to allow cached data to be used
         """
+        logging.info("Checking if we can re-use injection config file...")
         if os.path.isfile(self.config_file_name) is False:
-            logging.info("No config file {} found".format(self.config_file_name))
+            logging.info("No config file {} found.".format(self.config_file_name))
             return True
         else:
-            logging.info("Config file {} already exists".format(self.config_file_name))
+            logging.info("Config file {} already exists.".format(self.config_file_name))
 
         with open(self.config_file_name, "r") as f:
             file_content = f.read()
             if file_content == content:
                 logging.info(
-                    "File contents match, no update of {} required".format(
+                    "File contents match, no update of {} required.".format(
                         self.config_file_name
                     )
                 )
                 return False
             else:
                 logging.info(
-                    "File contents unmatched, updating {}".format(self.config_file_name)
+                    "File contents unmatched, updating {}.".format(self.config_file_name)
                 )
                 return True
 
diff --git a/tests.py b/tests.py
index a793f04..2b826b4 100644
--- a/tests.py
+++ b/tests.py
@@ -80,12 +80,16 @@ class Writer(Test):
         Writer = pyfstat.Writer(self.label, outdir=self.outdir, duration=3600)
         if os.path.isfile(Writer.sftfilepath):
             os.remove(Writer.sftfilepath)
+        # first run: make everything from scratch
         Writer.make_cff()
         Writer.run_makefakedata()
         time_first = os.path.getmtime(Writer.sftfilepath)
+        # second run: should re-use .cff and .sft
+        Writer.make_cff()
         Writer.run_makefakedata()
         time_second = os.path.getmtime(Writer.sftfilepath)
         self.assertTrue(time_first == time_second)
+        # third run: touch the .cff to force regeneration
         time.sleep(1)  # make sure timestamp is actually different!
         os.system("touch {}".format(Writer.config_file_name))
         Writer.run_makefakedata()
-- 
GitLab