Fix desimodel_sync script failure to update fp model

[dstndstn]

This turned out to be caused by an overly permissive regex in selecting the focalplane model files!

They wanted to allow yaml or json, optionally gzipped, so the regex is r"^desi-exclusion_(.*)\.(?:yaml|json).*$") …. BUT, the fp-sync script renames the previous exclusion file to something like desi-exclusion_2024-08-20T04:34:57+00:00.json.gz.previous …. which matches the regex!! And since the files are selected using os.walk, the one that matches the regex and appears last in the directory listing is the one chosen! So it was actually reading the previous exclusions list, which is why the updates weren’t working!

I also made some string (unicode) rather than bytes changes to table formats.

[coveralls]

coverage: 50.113% (-0.06%) from 50.17%
when pulling 6ae355c on fix-sync
into f668126 on main.

[schlafly]

Thanks Dustin. This looks good to me. I've also asked Ted to take a look. The regex change looks obvious and correct; I don't have any real sense for what kind of impact the unicode changes may have.

[schlafly]

Note that to use this, we'll have to:

• update the focal plane sync script to use this version, invoking some future release version here: https://github.com/desihub/desimodel/blob/main/etc/desimodel_sync_kpno_cron.sh#L47
• do a release of desimodel
• install the release at KPNO

[dstndstn]

Yeah, I don't think the unicode changes are actually required, just when I was running the code, in a bunch of places there was logging showing the old values (read from fp state files) were being interpreted as strings, while new data were being interpreted as byte arrays, so I just forced things to be strings upon reading.

[araichoor]

yes, it would be good to have Ted having a look.
as far as I can say, it looks ok to me, but I don t have in mind all the code architecture/philosophy...

[dstndstn]

Hi folks. It would be good to get this into production.

I tried applying just the regex fix, and if I compare the results of that run vs. this PR, I get the following differences:

In the logging, it prints out:

> diff -u log-re.log log-full.log
--- log-re.log	2024-09-12 10:42:02
+++ log-full.log	2024-09-12 10:40:29
@@ -36,7 +36,7 @@
     MAX_T:  188.83485412597656
     EXCLUSION:  affabed4677a3d9d
   NEW:
-    TIME:  b'2024-08-29T17:01:17+00:00'
+    TIME:  2024-08-29T17:01:17+00:00
     LOCATION:  7257
     STATE:  0
     POS_T:  0.0
@@ -45,7 +45,7 @@
     MAX_P:  182.0
     MIN_T:  -191.83485412597656
     MAX_T:  188.83485412597656
-    EXCLUSION:  b'a2326e0f3b61bbe7'
+    EXCLUSION:  a2326e0f3b61bbe7
 Location 7493:
   OLD:
     LOCATION:  7493
@@ -58,7 +58,7 @@
     MAX_T:  190.5631561279297
     EXCLUSION:  55081c0f0ecdc848
   NEW:
-    TIME:  b'2024-08-29T17:01:17+00:00'
+    TIME:  2024-08-29T17:01:17+00:00
     LOCATION:  7493
     STATE:  2
     POS_T:  134.67196655273438
@@ -67,7 +67,7 @@
     MAX_P:  182.0
     MIN_T:  -193.5631561279297
     MAX_T:  190.5631561279297
-    EXCLUSION:  b'55081c0f0ecdc848'
+    EXCLUSION:  55081c0f0ecdc848

and so on. To me, this looks like that without the bytes->strings changes, the code is receiving the data from the calib files as bytes, whereas it looks like the "existing" exclusions and timestamps are received as strings.

On the other hand, when I check the resulting desi_state-*.ecsv and desi-exclusion-*.json.gz files, they are identical.

So my opinion is that these bytes->strings changes are good form, but not strictly required to get us back up and running again.

cheers,
dustin

[schlafly]

Okay, this looks good to me. @sbailey , let's plan to tag this and install on the DESI cluster ~tomorrow?

[sybenzvi]

This is only a proposal to tag desimodel, not to make a new desiconda release, right? If so, we can upgrade the desiconda installation at KPNO (last updated in August 2023) to desiconda 2.2.0, and then upgrade the installation of desimodel to the new tag.

[schlafly]

Yes, sorry. No, we don't need a new desiconda. Yes, I think we only need a new tag. This
https://github.com/desihub/desimodel/blob/main/etc/desimodel_sync_kpno_cron.sh#L47
https://github.com/desihub/desimodel/blob/main/etc/desimodel_sync_kpno_force.sh#L47
should also get updated to point to that new tag, and then we need to actually update those files at Kitt Peak after we install the tag there. I don't expect we need to upgrade the desiconda, though we could.

[sybenzvi]

The tag at KPNO is updated. For my own documentation, here were my steps (as datasystems):

export DESICONDA_VERSION=default
export DESIMODULES=21.7e  # latest stable installed version at KPNO!
export DESI_PRODUCT_ROOT="/software/datasystems/desiconda/${desiconda}"

module load desimodules/${DESIMODULES}
desiInstall -r $DESI_PRODUCT_ROOT desimodel 0.19.2

I then edited desimodel_sync_kpno_cron.sh and desimodel_sync_kpno_force.sh in ~datasystems/desimodel_sync to swap desimodel to v0.19.2.