DeltaUnit for YearsBC not resolving offset correctly?

[benhutchison]

To experiment with the DeltaQuantity feature, I created a DeltaUnit for years BC (obviously only approximate given all the date irregularities in calendars). This differs from the Fahrenheit example in that both the sign and the offset of the unit are negative. When I convert 100BC back into an Instant, it shows as 1870. So. it appears as if the conversion factor, but not the offset, worked as expected.

import coulomb.*
import coulomb.syntax.*
import algebra.instances.all.given
import coulomb.ops.algebra.all.given
import coulomb.policy.standard.given
import scala.language.implicitConversions
import coulomb.units.time.{*, given}

object bc:
  final type YearsBC
  given ctx_unit_YearsBC: define.DeltaUnit[YearsBC, -31536000 * Second, -62133800000L, "years BC", "BC"] = define.DeltaUnit()

import bc.*
import coulomb.units.javatime.*
import coulomb.units.javatime.conversions.all.given

@main def testBC =
  println(new java.text.SimpleDateFormat("dd MM yyyy G").parse("01 01 100 BC").toInstant)

  //1870-01-25T00:00:00Z
  println(100.withEpochTime[YearsBC].toInstant)

[erikerlandson]

deleted my previous comment, it might be wrong

[erikerlandson]

Probably YearsBC violates my simplifying hack here:

https://github.com/erikerlandson/coulomb/blob/scala3/units/src/main/scala/coulomb/units/time.scala#L229

[benhutchison]

Hi Erik, Thanks for taking a looking and linking that code. It does look as if `toInstant` would be unable to emit any Instant prior to 1970, since it adds a Duration (a non-negative magnitude) to the epoch origin (being 1970): def toInstant(using q2d: QuantityDuration[V, U]): Instant = // taking advantage of Instant and EpochTime sharing same 1970 reference Instant.EPOCH.plus(q2d(epochTime.value.withUnit[U])) That doesnt seem in general to be a valid assumption since the time delta from epoch origin is a signed quantity and may be negative. It is also perplexing because my example is emitting an Instant at 1870.

[erikerlandson]

yes, the 1870 part is because I was (incorrectly) side-stepping the proper DeltaUnit conversions, which include offset. So it is subtracting 100 years from 1970. instead of 2070 years.

One other thing, the offset in your definition should be in years, so 1970:

given ctx_unit_YearsBC: define.DeltaUnit[YearsBC, -31536000 * Second, 1970, "years BC", "BC"] = define.DeltaUnit()

I have another minor type bug I think I need to fix as well. I may add your example as a unit test in a separate PR, to ensure this is working.

[benhutchison]

At a more conceptual level, I will mention that the issue with toInstant is a motivating example "in the wild" of something I advocated in my earlier video [https://youtu.be/lLUazJS-65k], namely that vectors and magnitudes should be separated in the type system. My mental model is that there are 4 different mathematical categories at play: - 1D Vectors (Coulomb's Quantity) - 1D Points (Coulomb's DeltaQuantity) and Vectors decompose into: - Non-negative Magnitudes - Directions (which in 1D are just signs) A type-level separation would prevent adding a magnitude to a point, without first converting the magnitude to a vector, which would require a direction. This would have shown up the bug in toInstant.

…

On Sun, Nov 13, 2022 at 7:48 AM Erik Erlandson ***@***.***> wrote: yes, the 1870 part is because I was (incorrectly) side-stepping the proper DeltaUnit conversions, which include offset. So it is subtracting 100 years from 1970. instead of 2070 years. One other thing, the offset in your definition should be in years, so 1970: given ctx_unit_YearsBC: define.DeltaUnit[YearsBC, -31536000 * Second, 1970, "years BC", "BC"] = define.DeltaUnit() I have another minor type bug I think I need to fix as well. I may add your example as a unit test in a separate PR, to ensure this is working. — Reply to this email directly, view it on GitHub <#383 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAXJZDC54PWJIJCHSNTIALWH77ABANCNFSM6AAAAAAR6EWVKY> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[erikerlandson]

I'm not very familiar with java time structures, or what their behavior is for "BC" but I think this might be right:

scala> 100d.withEpochTime[YearsBC].toInstant
val res1: java.time.Instant = -0099-05-18T00:00:00Z

[benhutchison]

Looks right to me. Is the only change here the use of Double, ie 100d?

[erikerlandson]

The main change is that I had to fix some of my earlier hackery. It works for Int as well.

scala> 100.withEpochTime[YearsBC].toInstant
val res2: java.time.Instant = -0099-05-18T00:00:00Z