Some cubes have invalid History BLOBs

[jessemapel]

ISIS version(s) affected: Tested under 4.4.0 & dev @ a2154a0 but this has more to do with what the cube you are using was processed with.

Description

Some of the history entries generates by various applications had/have issues where they are not PVL compliant and thus cannot be converted into a PVL object. This currently occurs when running cathist on these cubes because until this point, previous History data is kept as a Char * and not converted to a PVL object. Under the new BLOB refactor (#4082), the History class will now always convert all of its data to PVL. This is causing us to run into this problem much earlier, whenever we attempt to add a new History entry.

How to reproduce

Run cathist on a Cube with this error. An example can be found in the pca test data

cathist from=$ISISTESTDATA/isis/src/base/apps/pca/tsts/inverse/input/test.cub mode=brief
**ERROR** Error in PVL file on line [25].
**ERROR** Unable to read PVL keyword [FROM     = /work3/jdanton/testData/peaks.cub+1,2,3,7].
**ERROR** Keyword has extraneous data [,2,3,7] at the end.

Here is what the History blob looks like when dumped via blobdump

Object = stretch
  IsisVersion       = "3.0.11 beta | 2005-12-07"
  ProgramVersion    = 2003-07-29
  ExecutionDateTime = 2005-12-21T13:14:20
  HostName          = orkin
  UserName          = jdanton
  Description       = "Remaps pixel values in a cube"

  Group = UserParameters
    FROM = /work2/jdanton/testData/peaks_temp.cub
    TO   = /work2/jdanton/testData/peaks.cub
    LRS  = 0.0
  End_Group
End_Object

Object = crop
  IsisVersion       = "3.0.18 beta | 2006-05-12"
  ProgramVersion    = 2004-02-16
  ExecutionDateTime = 2006-05-18T13:50:21
  HostName          = orkin
  UserName          = jdanton
  Description       = "Crops a cube"

  Group = UserParameters
    FROM     = /work3/jdanton/testData/peaks.cub+1,2,3,7
    TO       = tmp.cub
    SAMPLE   = 100
    NSAMPLES = 512
    SINC     = 1
    LINE     = 512
    NLINES   = 512
    LINC     = 1
  End_Group
End_Object

Object = cubeatt
  IsisVersion       = "3.0.18 beta | 2006-05-12"
  ProgramVersion    = 2006-02-06
  ExecutionDateTime = 2006-05-18T13:50:30
  HostName          = orkin
  UserName          = jdanton
  Description       = "Cube attribute editor"

  Group = UserParameters
    FROM = /home/jdanton/isis/src/base/apps/pca/tmp.cub+1,2,3,7
    TO   = test.cub
  End_Group
End_Object

Possible Solution

Here are some possible solutions We have discussed:

1. We can change how we are refactoring History so that some of its data is stored in a Char * and new data is stored in a Pvl Object. This would retain the current functionality where you can add new History but when you try to access any of it, new or old, you get an error. It also would significantly complicate the History class.
2. Create an application that removes the History Blob from a Cube and saves it as a generic String Blob so that the History can be restarted cleanly. This would result in two Blobs on the Cube, a new "fresh" History blob that has only a record for this application being run and a "old history" blob that has the old corrupted history stored as just a String. The "old" history would be accessible via blobdump, but cathist would not be aware of it. We could also modify cathist to dump both the "old" history and the new History, but this would not work with mode=brief.
3. Do solution 2 automatically when we try to read History from a Cube.
4. Simply restart the History and delete the old corrupted History data. We can add an entry to the new History indicating that the History data was truncated.

The BLOB refactor team is leaning towards solution 3 because it preserves the old History and does not require additional effort from users. Solution 2 is a viable alternative because it is explicit, but it is unclear how prevalent this issue is and could require users to perform an additional step to do any processing of their data.

Additional context

See #1095 and #1535 for some previous work on this. That work addressed specific issues where individual applications were writing bad History entries, but did not address what to do with Cubes that already have the bad History attached to them.

[jlaura]

@jessemapel What is the timeline for making this decision?

[lwellerastro]

I'm definitely not for solution 4. We can't get rid of existing history entries. I literally depend on history entries to help me understand how old data were processed when there are no notes or print.prt files associated with cubes. We must keep history entries.

That only leaves options 2/3. I tend to use mode=brief the most because I usually just want executed commands, not all the other details. That being said, I'd take the long version of history over no history. I would vote for cathist to recognize old history so that it can at least supply that along with new history in the long version. I haven't used blobdump for much of anything and it's good to know we can dump history objects that way, but I would prefer we update cathist as opposed to having the user run a second program to get to image history (and somehow know they need to).

Sorry I never made comments on the original RFC, but I didn't know it was there. I looked on astrodiscuss and didn't see mention of it there either. I must have missed an announcement. I'll say I don't understand all/most of it to comment, but I do wonder how polygon will be impacted and how the number of applications that depend on it. But this wondering is mostly due to simply not understanding the details.

[jessemapel]

@jlaura We have to move forward with something in <1 week on this. We can come back and modify the functionality later though. For example we can get us to the current functionality under the BLOB work right now and then modify cathist to work with these things later.

[jessemapel]

I'm definitely not for solution 4. We can't get rid of existing history entries. I literally depend on history entries to help me understand how old data were processed when there are no notes or print.prt files associated with cubes. We must keep history entries.

I agree with this, but I want to point out that this will only happen for Cubes where you currently cannot access the Histories via cathist. Cubes with invalid History on them continue to have entries added to them, but that data is currently inaccessible outside of blobdump. The vast majority of cubes will be unaffected by this.

[lwellerastro]

Thanks for clarifying @jessemapel.

I may have run into this problem on a handful of occasions and I guess I just moved on because it didn't seem like there was anything I could do about it. Looking back at the links you supplied to old posts having history entry problems was useful and it's clear only certain types of applications may create an issue.

[jessemapel]

With the BLOB work, we preserved the existing functionality. We should still update cathist to handle cubes with these problems on them. Potentially we can just output the history as a string when we encounter this. Attempting to correct the bad keywords in the history PVL could require significant work as we'd have to check for a huge list of problems.

[github-actions]

Thank you for your contribution!

Unfortunately, this issue hasn't received much attention lately, so it is labeled as 'stale.'

If no additional action is taken, this issue will be automatically closed in 180 days.