Addressing PR comments by Copilot · Pull Request #1 · The-Interdependency/ucns

Copilot · 2026-04-25T04:44:47Z

Original prompt

this is a repo intended to have no dependencies and will use a novel unit circle number system to create embeddings that are faster and more efficient than currently available. files to follow shortly after creation.

…UCNEmbedding, similarity metrics, tests, docs Agent-Logs-Url: https://github.com/The-Interdependency/ucns/sessions/7fc7989f-a74b-41c2-abc5-5e7c722996cd Co-authored-by: wayseer00 <258336486+wayseer00@users.noreply.github.com>

Agent-Logs-Url: https://github.com/The-Interdependency/ucns/sessions/7fc7989f-a74b-41c2-abc5-5e7c722996cd Co-authored-by: wayseer00 <258336486+wayseer00@users.noreply.github.com>

chatgpt-codex-connector · 2026-04-25T05:14:32Z

To use Codex here, create a Codex account and connect to github.

Copilot

Pull request overview

Bootstraps a new zero-dependency ucns Python package implementing Unit Circle Number (UCN) primitives, FFT-based “epicycle” decomposition, Möbius/Poincaré disk geometry, and embedding + similarity APIs, along with an initial unittest suite and packaging/docs.

Changes:

Adds core UCNS modules (core, epicycle, mobius, embedding, similarity) and exports them via ucns/__init__.py.
Introduces a unittest test suite covering core functionality across modules.
Adds packaging (pyproject.toml), documentation (README.md), and repo hygiene (.gitignore).

Reviewed changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`ucns/core.py`	Implements `UCN` angle primitive + arithmetic, metrics, and uint16-style serialization.
`ucns/epicycle.py`	Pure-Python radix-2 FFT/ifft and `EpicycleDecomposition` with packing.
`ucns/mobius.py`	Möbius transform utilities + Poincaré distance and disk/circle projections.
`ucns/similarity.py`	Similarity/distance metrics for embeddings and epicycle amplitudes.
`ucns/embedding.py`	High-level `UCNEmbedding` encoder, similarity, packed storage, nearest search.
`ucns/__init__.py`	Public API surface / re-exports and version.
`tests/test_core.py`	Unittest coverage for `UCN` behaviors and serialization.
`tests/test_epicycle.py`	Unittest coverage for FFT/ifft, decomposition, packing, similarity.
`tests/test_mobius.py`	Unittest coverage for Möbius operations and Poincaré distance helpers.
`tests/test_embedding.py`	Unittest coverage for encoding, packing/unpacking, similarity, nearest.
`tests/test_similarity.py`	Unittest coverage for similarity/distance functions.
`tests/__init__.py`	Marks tests as a package (empty).
`pyproject.toml`	Adds packaging metadata and dependency-free configuration.
`README.md`	Adds library overview, usage examples, API reference, and test instructions.
`.gitignore`	Adds standard Python build/test/editor ignores.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T05:18:37Z

+
+[tool.setuptools.package-data]
+ucns = ["py.typed"]


[tool.setuptools.package-data] ucns = ["py.typed"] declares a PEP 561 marker file, but there is no ucns/py.typed in the package. Either add the marker file (and type hints) or remove this entry to avoid packaging/install inconsistencies.

Suggested change

[tool.setuptools.package-data]

ucns = ["py.typed"]

Copilot · 2026-04-25T05:18:37Z

+        # Numerically stable composition via a sample point
+        # We derive the new (a, phi) from where each transform sends 0.
+        # T_composed sends 0 → self(other(0))
+        a_new = self(other(0j))
+        # Determine rotation by evaluating at a second point
+        p = other(0.5 + 0j)
+        q = self(p)
+        # q = e^(i*phi_new) * (q_unnorm) → phi_new = arg(q / T_a_new(a_sample))
+        t_check = MobiusTransform(a_new)
+        sample = 0.5 + 0j
+        out = q
+        raw = (out - a_new) / (1.0 - a_new.conjugate() * out)


MobiusTransform.compose computes a_new = self(other(0j)), but per the class docstring a is the preimage of 0 (the transform maps a ↦ 0). Using the image of 0 will produce an incorrect composed transform. Consider deriving the composed (a, phi) analytically, or at minimum solve for a_new via (self ∘ other)(a_new) == 0 (i.e., a_new = other.inverse()(self.a) when using the current parameterization).

Suggested change

# Numerically stable composition via a sample point

# We derive the new (a, phi) from where each transform sends 0.

# T_composed sends 0 → self(other(0))

a_new = self(other(0j))

# Determine rotation by evaluating at a second point

p = other(0.5 + 0j)

q = self(p)

# q = e^(i*phi_new) * (q_unnorm) → phi_new = arg(q / T_a_new(a_sample))

t_check = MobiusTransform(a_new)

sample = 0.5 + 0j

out = q

raw = (out - a_new) / (1.0 - a_new.conjugate() * out)

# In this parameterization, `a` is the preimage of 0:

# T(a) = 0. Therefore for T = self ∘ other, `a_new` must satisfy

# self(other(a_new)) = 0, i.e. other(a_new) = self.a.

a_new = other.inverse()(self.a)

# Recover the rotation by evaluating the composition at any sample

# point in the disk distinct from `a_new`.

for sample in (0j, 0.5 + 0j, 0.5j, -0.5 + 0j):

if abs(sample - a_new) > 1e-12:

break

out = self(other(sample))

raw = out * (1.0 - a_new.conjugate() * sample) / (sample - a_new)

Copilot · 2026-04-25T05:18:38Z

+        # Numerically stable composition via a sample point
+        # We derive the new (a, phi) from where each transform sends 0.
+        # T_composed sends 0 → self(other(0))
+        a_new = self(other(0j))
+        # Determine rotation by evaluating at a second point
+        p = other(0.5 + 0j)
+        q = self(p)
+        # q = e^(i*phi_new) * (q_unnorm) → phi_new = arg(q / T_a_new(a_sample))
+        t_check = MobiusTransform(a_new)
+        sample = 0.5 + 0j
+        out = q
+        raw = (out - a_new) / (1.0 - a_new.conjugate() * out)
+        phi_new = cmath.phase(raw) if abs(raw) > 1e-15 else 0.0


compose() currently defines t_check and sample but never uses them, which makes the implementation harder to follow and suggests the computation is incomplete. Either remove these variables or use them as part of a verifiable computation of phi_new (and add a unit test that self.compose(other)(z) == self(other(z)) for representative z).

Suggested change

# Numerically stable composition via a sample point

# We derive the new (a, phi) from where each transform sends 0.

# T_composed sends 0 → self(other(0))

a_new = self(other(0j))

# Determine rotation by evaluating at a second point

p = other(0.5 + 0j)

q = self(p)

# q = e^(i*phi_new) * (q_unnorm) → phi_new = arg(q / T_a_new(a_sample))

t_check = MobiusTransform(a_new)

sample = 0.5 + 0j

out = q

raw = (out - a_new) / (1.0 - a_new.conjugate() * out)

phi_new = cmath.phase(raw) if abs(raw) > 1e-15 else 0.0

# Write each transform as (A*z + B) / (C*z + D), then compose

# by matrix multiplication and recover the canonical disk

# automorphism parameters.

lam_self = cmath.exp(1j * self.phi)

lam_other = cmath.exp(1j * other.phi)

a1, a2 = self.a, other.a

A1, B1, C1, D1 = lam_self, -lam_self * a1, -a1.conjugate(), 1.0 + 0j

A2, B2, C2, D2 = lam_other, -lam_other * a2, -a2.conjugate(), 1.0 + 0j

A = A1 * A2 + B1 * C2

B = A1 * B2 + B1 * D2

C = C1 * A2 + D1 * C2

D = C1 * B2 + D1 * D2

if abs(A) < 1e-15 or abs(D) < 1e-15:

raise ValueError("cannot compose transforms with degenerate coefficients")

a_new = -B / A

rotation = A / D

phi_new = cmath.phase(rotation) if abs(rotation) > 1e-15 else 0.0

Copilot · 2026-04-25T05:18:38Z

+    def to_int16(self) -> int:
+        """Quantise angle to an unsigned 16-bit integer (0 … 65 535).
+
+        Provides ~0.0001 rad (≈ 0.006°) angular resolution with only 2 bytes.
+        """
+        return min(65535, int(self._theta * 65535.0 / TAU))
+
+    @classmethod
+    def from_int16(cls, v: int) -> "UCN":
+        """Restore a UCN from a 16-bit quantised integer."""
+        return cls(int(v) * TAU / 65535.0)
+
+    def to_bytes(self) -> bytes:
+        """Serialise to 2 bytes (little-endian unsigned short)."""
+        return struct.pack("<H", self.to_int16())
+
+    @classmethod
+    def from_bytes(cls, data: bytes) -> "UCN":
+        """Deserialise from 2 bytes."""
+        (v,) = struct.unpack("<H", data[:2])
+        return cls.from_int16(v)


to_int16() / from_int16() operate on unsigned values in the range 0…65535 (uint16), but the name int16 commonly implies a signed 16-bit integer (−32768…32767). Renaming to to_uint16 / from_uint16 (or similar) would avoid confusion for API consumers.

Suggested change

def to_int16(self) -> int:

"""Quantise angle to an unsigned 16-bit integer (0 … 65 535).

Provides ~0.0001 rad (≈ 0.006°) angular resolution with only 2 bytes.

"""

return min(65535, int(self._theta * 65535.0 / TAU))

@classmethod

def from_int16(cls, v: int) -> "UCN":

"""Restore a UCN from a 16-bit quantised integer."""

return cls(int(v) * TAU / 65535.0)

def to_bytes(self) -> bytes:

"""Serialise to 2 bytes (little-endian unsigned short)."""

return struct.pack("<H", self.to_int16())

@classmethod

def from_bytes(cls, data: bytes) -> "UCN":

"""Deserialise from 2 bytes."""

(v,) = struct.unpack("<H", data[:2])

return cls.from_int16(v)

def to_uint16(self) -> int:

"""Quantise angle to an unsigned 16-bit integer (0 … 65 535).

Provides ~0.0001 rad (≈ 0.006°) angular resolution with only 2 bytes.

"""

return min(65535, int(self._theta * 65535.0 / TAU))

def to_int16(self) -> int:

"""Backward-compatible alias for :meth:`to_uint16`."""

return self.to_uint16()

@classmethod

def from_uint16(cls, v: int) -> "UCN":

"""Restore a UCN from an unsigned 16-bit quantised integer."""

return cls(int(v) * TAU / 65535.0)

@classmethod

def from_int16(cls, v: int) -> "UCN":

"""Backward-compatible alias for :meth:`from_uint16`."""

return cls.from_uint16(v)

def to_bytes(self) -> bytes:

"""Serialise to 2 bytes (little-endian unsigned short)."""

return struct.pack("<H", self.to_uint16())

@classmethod

def from_bytes(cls, data: bytes) -> "UCN":

"""Deserialise from 2 bytes."""

(v,) = struct.unpack("<H", data[:2])

return cls.from_uint16(v)

Copilot · 2026-04-25T05:18:38Z

@@ -0,0 +1,23 @@
+[build-system]
+requires = ["setuptools>=61"]
+build-backend = "setuptools.backends.legacy:build"


build-backend = "setuptools.backends.legacy:build" does not correspond to a standard setuptools PEP-517 backend entry point and will likely fail builds. Use a valid backend such as setuptools.build_meta (or setuptools.build_meta:__legacy__ if you explicitly need legacy behavior).

Suggested change

build-backend = "setuptools.backends.legacy:build"

build-backend = "setuptools.build_meta"

Copilot · 2026-04-25T05:18:38Z

+python -m pytest tests/ -v
+# or without pytest:


The "Running the tests" section suggests python -m pytest ..., but pytest isn't declared anywhere in pyproject.toml (even as an optional/dev dependency). Either remove the pytest command or add it under an extra (e.g. dev) so the README instructions are reproducible.

Suggested change

python -m pytest tests/ -v

# or without pytest:

Copilot · 2026-04-25T05:18:39Z

+    # ------------------------------------------------------------------
+
+    def __repr__(self) -> str:
+        return f"MobiusTransform(a={self.a:.4f}, phi={self.phi:.4f})"


__repr__ formats self.a with :.4f, but self.a is a complex and will raise TypeError: unsupported format string passed to complex.__format__. Use !r, or format real/imag parts separately, so repr() is safe.

Suggested change

return f"MobiusTransform(a={self.a:.4f}, phi={self.phi:.4f})"

return (

f"MobiusTransform(a={self.a.real:.4f}{self.a.imag:+.4f}j, "

f"phi={self.phi:.4f})"

)

Copilot · 2026-04-25T05:18:39Z

+    for ai, bi in zip(a, b):
+        za = radius * cmath.exp(1j * ai)
+        zb = radius * cmath.exp(1j * bi)
+        denom = 1.0 - zb.conjugate() * za
+        if abs(denom) < 1e-15:
+            d = 0.0
+        else:
+            rho = abs((za - zb) / denom)
+            rho = min(rho, 1.0 - 1e-15)
+            d = 2.0 * math.atanh(rho)


In the near-boundary numerical guard, if abs(denom) < 1e-15: d = 0.0 returns maximal similarity when the hyperbolic distance should actually be very large (radius close to 1 makes denom≈1-r^2). Instead of forcing d=0, clamp rho to 1-ε and compute d = 2*atanh(rho) (or explicitly return a large distance / similarity close to cos(large)).

Suggested change

for ai, bi in zip(a, b):

za = radius * cmath.exp(1j * ai)

zb = radius * cmath.exp(1j * bi)

denom = 1.0 - zb.conjugate() * za

if abs(denom) < 1e-15:

d = 0.0

else:

rho = abs((za - zb) / denom)

rho = min(rho, 1.0 - 1e-15)

d = 2.0 * math.atanh(rho)

eps = 1e-15

for ai, bi in zip(a, b):

za = radius * cmath.exp(1j * ai)

zb = radius * cmath.exp(1j * bi)

denom = 1.0 - zb.conjugate() * za

if abs(denom) < eps:

rho = 1.0 - eps

else:

rho = abs((za - zb) / denom)

rho = min(max(rho, 0.0), 1.0 - eps)

d = 2.0 * math.atanh(rho)

Copilot · 2026-04-25T05:18:39Z

+    """
+    n = len(spectrum)
+    if n == 0:
+        return []


ifft() documents that N must be a power of two, but it doesn't validate this before calling _fft_inplace. If a non-power-of-two spectrum is passed, the algorithm will silently produce incorrect results. Consider adding an explicit power-of-two check and raising ValueError with a clear message.

Suggested change

return []

return []

if n & (n - 1):

raise ValueError("ifft() requires spectrum length to be a power of two")

Copilot · 2026-04-25T05:18:39Z

+        """Deserialise from 2 bytes."""
+        (v,) = struct.unpack("<H", data[:2])


from_bytes() slices data[:2] and will (a) raise a low-level struct.error if fewer than 2 bytes are provided, and (b) silently ignore any trailing bytes if more than 2 are provided. Consider validating len(data) == 2 and raising ValueError with a clear message to make misuse easier to diagnose.

Suggested change

"""Deserialise from 2 bytes."""

(v,) = struct.unpack("<H", data[:2])

"""Deserialise from exactly 2 bytes."""

if len(data) != 2:

raise ValueError(f"UCN.from_bytes() requires exactly 2 bytes, got {len(data)}")

(v,) = struct.unpack("<H", data)

Initial plan

8da9a05

Copilot AI self-assigned this Apr 25, 2026

Copilot AI and others added 2 commits April 25, 2026 04:55

Add .gitignore, remove __pycache__ from tracking

326eacb

Agent-Logs-Url: https://github.com/The-Interdependency/ucns/sessions/7fc7989f-a74b-41c2-abc5-5e7c722996cd Co-authored-by: wayseer00 <258336486+wayseer00@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Add novel unit circle number system for embeddings~~ Add Unit Circle Number System (UCNS) — zero-dependency Python embedding library Apr 25, 2026

Copilot AI requested a review from wayseer00 April 25, 2026 04:58

Claude AI changed the title ~~Add Unit Circle Number System (UCNS) — zero-dependency Python embedding library~~ Addressing PR comments Apr 25, 2026

Copilot AI reviewed Apr 25, 2026

View reviewed changes

wayseer00 merged commit e68fd7e into main Apr 25, 2026
5 checks passed

Navigation Menu

Uh oh!

Addressing PR comments - #1

Addressing PR comments#1
wayseer00 merged 3 commits into
mainfrom
copilot/create-novel-unit-circle-embeddings

Copilot AI commented Apr 25, 2026 •

edited by Claude AI

Loading

Uh oh!

chatgpt-codex-connector Bot commented Apr 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Copilot AI Apr 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        # Numerically stable composition via a sample point
-        # We derive the new (a, phi) from where each transform sends 0.
-        # T_composed sends 0 → self(other(0))
-        a_new = self(other(0j))
-        # Determine rotation by evaluating at a second point
-        p = other(0.5 + 0j)
-        q = self(p)
-        # q = e^(i*phi_new) * (q_unnorm) → phi_new = arg(q / T_a_new(a_sample))
-        t_check = MobiusTransform(a_new)
-        sample = 0.5 + 0j
-        out = q
-        raw = (out - a_new) / (1.0 - a_new.conjugate() * out)
-        phi_new = cmath.phase(raw) if abs(raw) > 1e-15 else 0.0
+        # Write each transform as (A*z + B) / (C*z + D), then compose
+        # by matrix multiplication and recover the canonical disk
+        # automorphism parameters.
+        lam_self = cmath.exp(1j * self.phi)
+        lam_other = cmath.exp(1j * other.phi)
+        a1, a2 = self.a, other.a
+        A1, B1, C1, D1 = lam_self, -lam_self * a1, -a1.conjugate(), 1.0 + 0j
+        A2, B2, C2, D2 = lam_other, -lam_other * a2, -a2.conjugate(), 1.0 + 0j
+        A = A1 * A2 + B1 * C2
+        B = A1 * B2 + B1 * D2
+        C = C1 * A2 + D1 * C2
+        D = C1 * B2 + D1 * D2
+        if abs(A) < 1e-15 or abs(D) < 1e-15:
+            raise ValueError("cannot compose transforms with degenerate coefficients")
+        a_new = -B / A
+        rotation = A / D
+        phi_new = cmath.phase(rotation) if abs(rotation) > 1e-15 else 0.0

	build-backend = "setuptools.backends.legacy:build"
	build-backend = "setuptools.build_meta"

-        return f"MobiusTransform(a={self.a:.4f}, phi={self.phi:.4f})"
+        return (
+            f"MobiusTransform(a={self.a.real:.4f}{self.a.imag:+.4f}j, "
+            f"phi={self.phi:.4f})"
+        )

		"""Deserialise from 2 bytes."""
		(v,) = struct.unpack("<H", data[:2])

-        """Deserialise from 2 bytes."""
-        (v,) = struct.unpack("<H", data[:2])
+        """Deserialise from exactly 2 bytes."""
+        if len(data) != 2:
+            raise ValueError(f"UCN.from_bytes() requires exactly 2 bytes, got {len(data)}")
+        (v,) = struct.unpack("<H", data)

Uh oh!

Conversation

Copilot AI commented Apr 25, 2026 • edited by Claude AI Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Apr 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Apr 25, 2026 •

edited by Claude AI

Loading