feat: Add Elias-Fano encoding by limakzi · Pull Request #235 · gap-packages/smallantimagmas

limakzi · 2026-03-06T00:58:16Z

No description provided.

Copilot

Pull request overview

This PR updates how small antimagmas’ multiplication tables are serialized in package metadata by switching from the old Convert/Reverse representation to a new packed integer encoding, and wires the new decode path into the public constructors.

Changes:

Add cached helpers plus MultiplicationTableEncode/Decode/GetEntry for base-(n^n) packed integer encoding of multiplication tables.
Update SmallAntimagma, AllSmallAntimagmas, and ReallyAllSmallAntimagmas to decode metadata using the new encoding.
Add a dedicated test suite for the new encoding and update the packaged .g.gz metadata files accordingly.

Reviewed changes

Copilot reviewed 4 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`lib/helper.gd`	Adds encoding/decoding helpers and caching used to interpret metadata as packed integers.
`lib/smallantimagmas.gi`	Switches magma construction to use `MultiplicationTableDecode(..., order)` on metadata entries.
`lib/utils.gi`	Minor refactor to name the tuples enumerator before filtering.
`tst/test_helper_multiplication_table_encoding.tst`	New tests for caches, encode/decode round-trips, injectivity, and old/new equivalence.
`data/non-isomorphic/{2,3,4}/small_*.g.gz`	Updated non-isomorphic metadata content to match the new encoding format.
`data/all/{2,3}/small_*.g.gz`	Updated “all” metadata content to match the new encoding format.

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

Copilot · 2026-03-06T10:08:21Z

@limakzi I've opened a new pull request, #236, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-03-06T10:08:38Z

@limakzi I've opened a new pull request, #237, to work on those changes. Once the pull request is ready, I'll request review from you.

olexandr-konovalov · 2026-07-14T23:39:24Z

@limakzi oops, why CI fails now...

codecov · 2026-07-15T07:33:26Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.84%. Comparing base (e70d952) to head (852f930).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #235      +/-   ##
==========================================
+ Coverage   90.96%   91.84%   +0.87%     
==========================================
  Files          10       10              
  Lines         299      331      +32     
==========================================
+ Hits          272      304      +32     
  Misses         27       27

Files with missing lines	Coverage Δ
data/2/small_2.g.gz	`100.00% <ø> (ø)`
data/3/small_3.g.gz	`100.00% <ø> (ø)`
data/4/small_4.g.gz	`100.00% <ø> (ø)`
lib/helper.gd	`84.41% <100.00%> (+11.08%)`	⬆️
lib/properties.gd	`100.00% <ø> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

olexandr-konovalov

The code seems to work, but maybe one should add more safety mechanisms to check for the allowed values of arguments?

olexandr-konovalov · 2026-07-16T19:38:38Z

@@ -0,0 +1,100 @@
+gap> START_TEST("test_helper_tables_encode_decode.tst");


I think this is quite thorough testing, with lots of corner cases and property checks. Should the description be here or moved closer to the code in lib/helper.gd though?

olexandr-konovalov · 2026-07-16T19:39:31Z

+gap> __SmallAntimagmaHelper.TablesDecode(2, [7, 3, 2]);
+[ [ 2, 4 ], [ 3, 3 ], [ 4, 1 ] ]
+
+# Duplicates survive encoding as deltas equal to 0.


This is not a continuation of Step 4 - maybe think of another header?

In general, we will move these comments to helper.gd, the comments about the test MUST document the test-case.

olexandr-konovalov · 2026-07-16T19:48:00Z

+# Step 1: a table in row form is a base-(n^n) number.
+# For n = 2 the base is 2^2 = 4, digits are (row index - 1).
+# [3, 3] -> (3 - 1) * 4 + (3 - 1) = 10, first delta is taken from 0.
+gap> __SmallAntimagmaHelper.TablesEncode(2, [[3, 3]]);


Should there be any input control? What happens if the number is larger than n^n?

gap> __SmallAntimagmaHelper.TablesEncode(2, [[5]]); [ 4 ] gap> __SmallAntimagmaHelper.TablesDecode(2, [4]); [ [ 2, 1 ] ]

Some other anomalies:

gap> __SmallAntimagmaHelper.TablesEncode(2, [[3,0]]); [ 7 ] gap> __SmallAntimagmaHelper.TablesDecode(2, [7]); [ [ 2, 4 ] ]

and

gap> __SmallAntimagmaHelper.TablesEncode(2, [[0,3]]); [ -2 ] gap> __SmallAntimagmaHelper.TablesDecode(2, [-2]); [ [ 1, -1 ] ]

As we discussed, its not surprising it works, but we will implement validation.
For encoding - yes; for decoding - nice to have, but not necessary.

olexandr-konovalov · 2026-07-16T19:48:35Z

+> __SmallAntimagmaHelper.getSmallAntimagmaMetadata(n)()));
+true
+
+gap> STOP_TEST("test_helper_tables_encode_decode.tst");


Maybe add checks that an error is produced when it's expected?

Copilot AI review requested due to automatic review settings March 6, 2026 00:58

github-actions Bot added the feature label Mar 6, 2026

Copilot started reviewing on behalf of limakzi March 6, 2026 00:58 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

Comment thread lib/helper.gd Outdated

Comment thread lib/helper.gd Outdated

Comment thread lib/helper.gd Outdated

Comment thread lib/helper.gd Outdated

Comment thread lib/helper.gd Outdated

limakzi changed the title ~~feat: Change encoding~~ feat: Add Elies-Fano encoding Mar 6, 2026

limakzi changed the title ~~feat: Add Elies-Fano encoding~~ feat: Add Elias-Fano encoding Mar 6, 2026

Copilot AI mentioned this pull request Mar 6, 2026

Fix typo: "ambigous" → "ambiguous" in error messages #236

Merged

Copilot AI mentioned this pull request Mar 6, 2026

Replace RowDictWithCache with on-demand RowFromId to fix memory exhaustion #237

Draft

olexandr-konovalov reviewed Mar 6, 2026

View reviewed changes

Comment thread lib/helper.gd Outdated

limakzi requested a review from olexandr-konovalov July 10, 2026 09:32

limakzi force-pushed the feature/change-encoding branch from abdffc4 to 717ecdf Compare July 14, 2026 21:26

limakzi force-pushed the feature/change-encoding branch from 717ecdf to 59de591 Compare July 15, 2026 07:29

feat: Change encoding for tables

333d8a5

limakzi force-pushed the feature/change-encoding branch from 59de591 to 333d8a5 Compare July 15, 2026 07:38

chore: Change path and remove non-isomorphic dir

852f930

olexandr-konovalov reviewed Jul 16, 2026

View reviewed changes

		@@ -0,0 +1,100 @@
		gap> START_TEST("test_helper_tables_encode_decode.tst");

Uh oh!

Conversation

limakzi commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Mar 6, 2026

Uh oh!

Copilot AI commented Mar 6, 2026

Uh oh!

Uh oh!

olexandr-konovalov commented Jul 14, 2026

Uh oh!

codecov Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

olexandr-konovalov left a comment

Choose a reason for hiding this comment

Uh oh!

olexandr-konovalov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

olexandr-konovalov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

limakzi Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

olexandr-konovalov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

olexandr-konovalov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

limakzi Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

olexandr-konovalov Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Jul 15, 2026 •

edited

Loading