Magic double values to support nulls in arrays of double in thrift

[nikhil-zlai]

Summary

we cannot represent absent data in time series with null values because the thrift serializer doesn't allow nulls in list - so we create a magic double value (based on string "chronon") to represent nulls

Checklist

• Added Unit Tests
• Covered by existing CI
• Integration tested
• Documentation update

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced a new constant magicNullDouble for handling null value representations

• Bug Fixes

  • Improved null value handling in serialization and data processing workflows

• Tests

  • Added new test case for TileDriftSeries serialization to validate null value management

The changes enhance data processing consistency and provide a standardized approach to managing null values across the system.

[coderabbitai]

Walkthrough

This pull request introduces a new constant magicNullDouble in the Chronon API, which is used to standardize the handling of null values across different components. The changes span multiple files, including the API constants, a new test class for tile series serialization, and modifications to pivot utilities. The primary goal appears to be creating a consistent approach to representing null values in numerical data structures.

Changes

File	Change Summary
api/src/main/scala/ai/chronon/api/Constants.scala	Added magicNullDouble constant with value -27980863399423854.0
api/src/test/scala/ai/chronon/api/test/TileSeriesSerializationTest.scala	New test class for TileDriftSeries serialization, verifying null value handling
online/src/main/scala/ai/chronon/online/stats/PivotUtils.scala	Modified doubleIterator to return Constants.magicNullDouble instead of null

Possibly related PRs

• Summary upload #50: Changes in SummaryUploader.scala involving null value handling

Suggested reviewers

• piyush-zlai

Poem

🧊 Null's Cold Embrace 🧊

In the realm of data's dance,
A magic number takes its stance
Replacing nulls with grace and might
-27980863399423854 shines bright!
No more void, no empty space

Warning

Review ran into problems

🔥 Problems

GitHub Actions: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository.

Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings.

---

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

api/src/test/scala/ai/chronon/api/test/TileSeriesSerializationTest.scala (1)

17-23: Consider using a helper method for null mapping.

Extract the null-to-magic-value conversion logic into a reusable helper method.

+ private def toMagicDouble(v: Double): JDouble =
+   if (v == null) Constants.magicNullDouble
+   else v.asInstanceOf[JDouble]

- val percentileDrifts: Seq[JDouble] = Seq(0.1, null, 0.3, null, 0.5, null, 0.7, null, 0.9)
-   .map(v =>
-     if (v == null)
-       Constants.magicNullDouble
-     else
-       v.asInstanceOf[JDouble]
-   )
+ val percentileDrifts: Seq[JDouble] = Seq(0.1, null, 0.3, null, 0.5, null, 0.7, null, 0.9)
+   .map(toMagicDouble)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 225e5af and c39f205.

📒 Files selected for processing (3)

• api/src/main/scala/ai/chronon/api/Constants.scala (1 hunks)
• api/src/test/scala/ai/chronon/api/test/TileSeriesSerializationTest.scala (1 hunks)
• online/src/main/scala/ai/chronon/online/stats/PivotUtils.scala (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (2)

• GitHub Check: no_spark_scala_tests
• GitHub Check: scala_compile_fmt_fix

🔇 Additional comments (1)

online/src/main/scala/ai/chronon/online/stats/PivotUtils.scala (1)

136-138: 🛠️ Refactor suggestion

Add validation for magic value collisions.

Ensure input values don't collide with magicNullDouble.

   def doubleIterator(isSetFunc: TileDrift => Boolean, extract: TileDrift => Double): Iterator[JDouble] = {
     drifts.iterator.map { drift =>
+      // Validate that actual values don't collide with magicNullDouble
+      def validateValue(v: Double): Unit =
+        require(v != Constants.magicNullDouble, s"Value $v collides with magicNullDouble")
+
       if (isSetFunc(drift)) {
-        JDouble.valueOf(extract(drift))
+        val value = extract(drift)
+        validateValue(value)
+        JDouble.valueOf(value)
       } else {
         Constants.magicNullDouble
       }
     }
   }

[ken-zlai]

Thanks for pairing on this @nikhil-zlai!