crotwell / seisplotjs

Javascript modules for parsing, manipulating and plotting seismic data.
http://crotwell.github.io/seisplotjs/
MIT License
61 stars 7 forks source link

miniseed.ts incorrect data conversion in toDateTime function #30

Closed rbeerster closed 8 months ago

rbeerster commented 9 months ago

While working with the Tutorial 6: Helicorder, I wanted to plot this SCNL: N4.N49A.00.LHZ. This results in a console log error: tutorial6.js:52 Assertion failed: Error: Unknown datetime argument: undefined, of type undefined at friendlyDateTime (seisplotjs_3.1.1_standalone.mjs:56247:11) at _Interval.fromDateTimes (seisplotjs_3.1.1_standalone.mjs:52808:24) at _Interval.union (seisplotjs_3.1.1_standalone.mjs:53138:22) at seisplotjs_3.1.1_standalone.mjs:59931:25 at Array.reduce () at _Seismogram.findStartEnd (seisplotjs_3.1.1_standalone.mjs:59930:31) at new _Seismogram (seisplotjs_3.1.1_standalone.mjs:59901:27) at merge (seisplotjs_3.1.1_standalone.mjs:61512:10) at seisplotjs_3.1.1_standalone.mjs:61565:47 at Map.forEach ()

After some digging around it appear that this line in toDataTime:

millisecond: Math.round(this.tenthMilli / 10)},

should actually be:

millisecond: Math.round(this.tenthMilli / 1000)},

since the output of msi -n 1 /var/tmp/fdsnws-dataselect_2023-12-13t19_47_43z.mseed (obtained from the URL in the console window) shows:

N4_N49A_00_LHZ, 000546, M, 512, 2 samples, 1 Hz, 2023,346,20:00:00.000000

where the tenthMilli is .000000 so dividing by 1000 would yield .000

If you agree, I can create a PR unless you just want to handle this. Thanks again for the seisplotjs module.

crotwell commented 9 months ago

Humm, I don't think this is your issue. The miniseed2 format stores times with a 4 decimal digits for fractional seconds, so between 0 and 9999. Luxon can only do millisecond precision, so the /10 is to convert "tenth milliseconds" into milliseconds. While the output of msi shows 6 digits, eg microseconds, this is not the precision actually stored in the data file.

That said, the undefined error you are seeing suggests something is wrong. If you can provide some more detail on the exact data you are looking at (perhaps the url you mention) then I can try to look into it.

rbeerster commented 9 months ago

Philip,

I have often said that, just because it works does not mean it is correct. That is why I raised the issue instead of doing a PR :-). Here is the URL from the console:

http://service.iris.edu/fdsnws/dataselect/1/query?net=N4&sta=N49A&loc=00&cha=LHZ&starttime=2023-12-13T02%3A00%3A00.000&endtime=2023-12-14T02%3A00%3A00.000&format=miniseed

Basically, I just took the tutorial6.js code and made these changes:

@@ -20,8 +21,8 @@ new sp.fdsndatacenters.DataCentersQuery() // snip start seismogram .then((dataSelectArray) => { return dataSelectArray[0]

I hope this helps.

On Wed, Dec 13, 2023 at 8:13 PM Philip Crotwell @.***> wrote:

Humm, I don't think this is your issue. The miniseed2 format stores times with a 4 decimal digits for fractional seconds, so between 0 and 9999. Luxon can only do millisecond precision, so the /10 is to convert "tenth milliseconds" into milliseconds. While the output of msi shows 6 digits, eg microseconds, this is not the precision actually stored in the data file.

That said, the undefined error you are seeing suggests something is wrong. If you can provide some more detail on the exact data you are looking at (perhaps the url you mention) then I can try to look into it.

— Reply to this email directly, view it on GitHub https://github.com/crotwell/seisplotjs/issues/30#issuecomment-1854946314, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG52UQESE6BDAAA257ORXITYJJHFHAVCNFSM6AAAAABAUAL23CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJUHE2DMMZRGQ . You are receiving this because you authored the thread.Message ID: @.***>

-- Bob Beer @.***>

crotwell commented 9 months ago

Luxon doesn't like milliseconds >= 1000, so miniseed records with fractional seconds of 9999 cause Invalid DateTime.

fixed in commit 198a804 will try to push out new release soon

rbeerster commented 9 months ago

Philip,

Luxon also does not like seconds to be equal to 60. So if this.sec = 59 and this.tenthMilli = 9999 you end up adding 1 second to this.sec and Luxon complains since it is now 60 . Here is an incomplete diff:

@@ -48083,6 +48084,20 @@ var BTime = class {
   toDateTime() {
     let millis = Math.round(this.tenthMilli / 10);
     const addSec = Math.floor(millis / 1e3);
+    if (addSec == 1 && this.sec == 59) {
+      this.sec = 0;
+      if (this.min != 59) {
+        this.min++;
+      } else {
+        this.min = 0;
+        if (this.hour != 23) {
+          this.hour++;
+        } else {
+          this.hour = 0;
+          this.jday++
+        }
+      }      
+    }
     millis = millis - 1e3 * addSec;
     return DateTime.fromObject(
       {
@@ -48090,7 +48105,7 @@ var BTime = class {
         ordinal: this.jday,
         hour: this.hour,
         minute: this.min,
-        second: this.sec + addSec,
+        second: this.sec ,
         millisecond: millis
       },
       UTC_OPTIONS

You will need to account for the year too.

crotwell commented 8 months ago

Ugg, you are right. Thanks for the bug report.

rbeerster commented 8 months ago

Philip,

How about just sidestepping the issue. Would this work:

toDateTime() {
    return DateTime.fromObject(
      {
        year: this.year,
        ordinal: this.jday,
        hour: this.hour,
        minute: this.min,
        second: this.sec ,
        millisecond: Math.floor(this.tenthMilli / 10),
      },
      UTC_OPTIONS);
  }

That is to say instead of round just use floor. That way 999 milliseconds stays at 999 milliseconds and you don't have to worry about any cascading date increments.

crotwell commented 8 months ago

made a change to allow luxon to handle the math of adding millis, so should increment seconds field correctly if millis rounds to 1000

See 7cef15f1b3f468e70da31c0ba2f4b456ee30df0d for fix

Please test and let me know if you think this fixes it.

Thanks again for bug reports

rbeerster commented 8 months ago

This appears to have corrected the invalid date issue. I Tested this on N4.N49A.00.LHZ, N4.Q51A.00.LHZ, and N4.Q52A.00.LHZ using the tutorial6.js.