Skip to content

Improve BinaryCIF float chain encoding#52

Open
yusufm99 wants to merge 3 commits into
rcsb:dev-bcif-stagingfrom
yusufm99:dev-bcif-chain-encoding
Open

Improve BinaryCIF float chain encoding#52
yusufm99 wants to merge 3 commits into
rcsb:dev-bcif-stagingfrom
yusufm99:dev-bcif-chain-encoding

Conversation

@yusufm99

Copy link
Copy Markdown

Optimize BinaryCIF float encoding with FixedPoint chain selection

Summary

This PR improves BinaryCIF serialization of floating-point columns in mmcif/io/BinaryCifWriter.py.

The production writer previously encoded float columns directly as ByteArray. This change adds BinaryCIF FixedPoint encoding followed by integer compression chains. It uses specific encoding rules for known high-volume structural fields and automatically selects the smallest supported chain for other float columns.

The final writer was tested on these 10 structures:

  • PDB: 2BVK, 4HHB, 12GB, 4BTS, and 3J3Q
  • IHM: 9A01, 9A25, and 9A8K
  • AlphaFold: AF_AFA0A017SEY2F1
  • ModelCIF: MA_MABAKCEPC0001

File-size comparison

The BCIF file sizes for all 10 structures were added together for each writer:

  • Production writer: 167,044,307 bytes
  • Final writer: 48,187,201 bytes
  • Total space saved: 118,857,106 bytes
  • Overall size reduction: 71.153%

The size reduction was calculated as:

(production total size - final total size)
÷ production total size
× 100

Serialization-time comparison

Each structure was serialized five times with each writer. For each structure, the median time from the five measured runs was selected. The median times for all 10 structures were then added together:

Production writer: 45.756497 seconds
Final writer: 61.785365 seconds
Overall time increase: 35.031%

The time increase was calculated as:

(final total of the 10 median times - production total of the 10 median times)
÷ production total of the 10 median times
× 100

The change is intentionally limited to one production file. It does not change the decoder, public writer API, dictionary loading, BinaryCIF version metadata, or package dependencies.

Motivation

BinaryCIF is column-oriented and supports chaining encoders such as FixedPoint, Delta, RunLength, IntegerPacking, and ByteArray. The existing py-mmcif writer encoded float columns directly as ByteArray. This is valid, but it does not take advantage of the additional compression available for coordinates and other structured numeric columns.

This implementation follows the BinaryCIF encoding approach used in Mol* and supported by the ciftools-java library:

  1. convert eligible float values into scaled integers using FixedPoint;
  2. apply integer encoders that match the data pattern in each column;
  3. fall back to numeric float ByteArray encoding when FixedPoint cannot be used safely.

The goal is to produce substantially smaller BCIF files while preserving the intended numeric precision and ensuring that the files can still be correctly read by other BinaryCIF implementations, including Mol* and ciftools-java.

Scope

  • Base branch: rcsb/py-mmcif:dev-bcif-staging
  • Head branch: yusufm99/py-mmcif:dev-bcif-chain-encoding
  • Commit: 096e052ba56353c6b867cacb175824dc8a7174f4
  • Files changed: 1
  • Diff: 400 additions, 20 deletions
  • Modified file: mmcif/io/BinaryCifWriter.py

No benchmark scripts, generated BCIF files, logs, backup writers, or local paths are included in the commit.

What changed and why

This section documents both the implementation and the reasoning behind each decision.

The implementation separates two kinds of behavior:

  1. Behavior derived directly from the BinaryCIF and Mol model*, including FixedPoint metadata, factor detection through four decimal places, the four integer-chain candidates, factor-1,000 coordinate encoding, and JavaScript-compatible rounding.
  2. Project-specific decisions derived from the py-mmcif data and experiments, including the expanded coordinate-item list, factor-10,000 anisotropic-U handling, the _ihm_sphere_obj_site.object_radius rule, the explicit RunLength hints, overflow fallback, feature switches, and the decision to keep StringArray fallback disabled.

The internal evidence paths below refer to the evidence package generated for repository commit:

096e052ba56353c6b867cacb175824dc8a7174f4

1. Pass column identity into the float encoder

Problem

The production float path received the values and mask for a column, but it did not receive the category or attribute name.

Without that context, the writer could not apply a deliberate precision or encoding policy to known structural fields. Coordinates, anisotropic displacement values, sphere radii, repeated B-factor fields, and unrelated general-purpose float columns all reached the same direct float ByteArray path.

External basis

Mol* supports encoding-strategy hints that identify a field by category and column name and optionally specify its float precision:

Change

The internal call path now forwards catName and atName:

BinaryCifWriter.serialize()
→ __encodeColumnData()
→ BinaryCifEncoders.encodeWithMask()
→ floatArrayMaskedEncoder()

The float encoder normalizes those values into an item name of the form:

_category.attribute

This allows it to select a specialized policy when one exists and the general float classifier otherwise.

The public interface remains unchanged:

serialize(filePath, containerList)

No constructor argument or public method signature was added.

Validation

The final writer passed:

  • the two existing targeted BinaryCIF writer tests;
  • the full repository suite of 114 tests, with only the three existing dictionary-test skips;
  • 37 CIF → BCIF → CIF comparisons with no structural, mask, value, or raw-type mismatches.

Result and decision

Column context is retained as an internal implementation detail because it enables precision and chain selection without changing the writer API or changing how dictionary types are assigned.

Internal evidence:

01_final_source/BinaryCifWriter_final.py
02_python_tests/testBinaryCifWriter_final.log
02_python_tests/full_test_suite_final.log
03_round_trip/roundtrip_totals.txt

2. Add a typed FixedPoint encoder and track type changes through the chain

Problem

The production writer encoded dictionary-typed float columns directly as float ByteArray data.

That representation is valid, but it does not allow the writer to exploit the lower dynamic range and repeated patterns that become available after decimal float values are converted into scaled integers.

A second issue was type propagation. Once FixedPoint transforms floats into integers, all later encoders must treat the working array as integer data. Continuing to pass the original float type to ByteArray would produce incorrect encoding metadata and packing behavior.

External basis

BinaryCIF defines FixedPoint as a float-to-Int32 transformation containing:

  • kind;
  • factor;
  • original float srcType.

See:

Mol* performs the conversion using Math.round:

The Mol* source also contains a TODO requesting a better overflow check before values are committed to an Int32Array.

Change

A typed fixedPointEncoderTyped() implementation now:

  1. accepts only float_32 or float_64 source data;
  2. multiplies every value by the selected integer factor;
  3. rounds using JavaScript Math.round-compatible behavior;
  4. verifies that every result is in the signed 32-bit range;
  5. returns a TypedArray with data type integer_32;
  6. records the BinaryCIF FixedPoint metadata.

The rounding helper uses:

math.floor(value + 0.5)

This reproduces the relevant JavaScript Math.round behavior, including its treatment of negative half values.

The generic chain executor now tracks the working data type. After FixedPoint, later Delta, RunLength, IntegerPacking, and ByteArray stages operate on integer data rather than on the original float type.

Safety behavior

FixedPoint is not used when:

  • a value is NaN;
  • a value is positive or negative infinity;
  • scaling would exceed the signed int32 range;
  • a requested chain raises an encoding exception.

Those cases use the established numeric float ByteArray path.

Validation

The focused fallback test verified five direct cases:

coordinate_nan               → ByteArray
coordinate_positive_infinity → ByteArray
coordinate_negative_infinity → ByteArray
coordinate_int32_overflow    → ByteArray
normal_coordinate            → FixedPoint, Delta, IntegerPacking, ByteArray

The 37-structure round-trip comparison then checked 143,178,047 values and reported:

Structural mismatches: 0
Mask mismatches:       0
Value mismatches:      0
Raw type mismatches:   0
Maximum absolute error: 0

All 37 generated files also passed the independent Java encoding verifier.

Result and decision

Typed FixedPoint encoding is enabled by default. Explicit finite-value and signed-int32 checks were retained because they make failure behavior deterministic and address a safety concern that the Mol* classifier itself identifies.

Internal evidence:

01_final_source/BinaryCifWriter_final.py
02_python_tests/focused_float_fallback_test_final.log
03_round_trip/roundtrip_totals.txt
04_java_verifier/final_pr_encoding_verifier_summary.txt

3. Detect a safe FixedPoint factor for general float columns

Problem

A fixed factor cannot be safely applied to every float column.

Some columns contain integer-valued floats, some require one to four decimal places, and some require more precision than the FixedPoint policy supports. The writer therefore needed a bounded rule for determining when a general float column can be converted to integers without blindly applying a factor.

External basis

The Mol* float classifier:

  • examines at most four decimal places;
  • uses a tolerance of 1e-6;
  • returns to ByteArray when a safe conversion cannot be established.

See:

Change

For general float columns, the writer searches these factors in ascending order:

1
10
100
1,000
10,000

For every value, it tests whether the scaled value lies within 1.0e-6 of an integer.

The greatest number of decimal places required by any value in the column determines the factor:

Required precision Factor
Integer-valued floats 1
One decimal place 10
Two decimal places 100
Three decimal places 1,000
Four decimal places 10,000

The detector returns no factor when:

  • any unmasked value is non-finite;
  • any value requires more than four detectable decimal places;
  • no tested factor satisfies the tolerance.

A separate signed-int32 check is then performed on the fully scaled column before the selected encoding is accepted.

Known coordinate items are intentionally different: they retain the explicit factor-1,000 coordinate policy rather than deriving their factor from the individual values in each file.

Validation

The final round-trip and interoperability tests validate the resulting factors across the complete 37-structure corpus.

The final evidence package does not contain a standalone unit-test table exercising factor 1, 10, 100, 1,000, and 10,000 individually. The factor order, tolerance, and bounded four-decimal behavior are directly visible in the final source, while the integration evidence verifies the resulting output across real PDB, IHM, AlphaFold, and ModelCIF data.

Result and decision

Automatic factor detection is used for general float columns and for the repeated-field hints whose precision is not fixed in advance.

The search remains bounded at four decimal places to match the adopted Mol* classification model and to prevent uncontrolled integer growth.

Internal evidence:

01_final_source/BinaryCifWriter_final.py
03_round_trip/roundtrip_summary.tsv
04_java_verifier/final_pr_encoding_verifier.log

4. Compare four integer chains for general FixedPoint columns

Problem

After a float column is converted to integers, no single integer chain is best for every data pattern.

For example:

  • direct IntegerPacking can be best for irregular low-range data;
  • RunLength can help repeated values;
  • Delta can help gradually changing values;
  • Delta followed by RunLength can help repeated differences.

Always forcing one chain would improve some columns while making others larger.

External basis

Mol* considers the same four integer strategies:

  1. IntegerPacking
  2. RunLength → IntegerPacking
  3. Delta → IntegerPacking
  4. Delta → RunLength → IntegerPacking

See:

Change

When a safe factor is available, py-mmcif evaluates these candidates in this order:

FixedPoint → IntegerPacking → ByteArray
FixedPoint → RunLength → IntegerPacking → ByteArray
FixedPoint → Delta → IntegerPacking → ByteArray
FixedPoint → Delta → RunLength → IntegerPacking → ByteArray

The candidate chains reuse py-mmcif’s existing Delta and RunLength encoders. These pre-existing encoders decline to transform columns with 40 or fewer values, and RunLength also declines when its value/count representation would contain more integers than the input. The threshold of 40 already exists in the current py-mmcif BinaryCifWriter script and was not introduced or independently selected by this change.

Validation

The controlled benchmark validates the complete final classifier against the production writer on ten structures under identical conditions.

Across the complete ten-structure set:

Production total BCIF size: 167,044,307 bytes
Final total BCIF size:       48,187,201 bytes
Bytes saved:                118,857,106 bytes
Overall size reduction:          71.153%

The additional classification work increased serialization time:

Production sum of medians: 45.756497 seconds 
Final sum of medians: 61.785365 seconds 
Overall time increase: 35.031%

These numbers measure the complete final implementation. They do not isolate the contribution of the four-chain classifier from the specialized hints.

Result and decision

The four-chain comparison was retained because it provides broad automatic coverage and produced a substantial aggregate size reduction, with the serialization-time increase documented as the principal tradeoff.

Internal evidence:

08_final_benchmark/benchmark_runs.tsv
08_final_benchmark/benchmark_summary.tsv
08_final_benchmark/benchmark_aggregate.tsv
08_final_benchmark/benchmark_metadata.txt

5. Use factor 1,000 and a Delta-based chain for known coordinates

Problem

Coordinate columns are among the largest numeric columns in structural data. Encoding them directly as 32- or 64-bit floating-point byte arrays misses substantial compression.

Allowing each coordinate column to infer its own precision would also make the stored coordinate precision depend on the particular values present in the file rather than on one deliberate coordinate policy.

External basis

Mol* exposes a predefined factor-1,000 FixedPoint coordinate-style chain:

FixedPoint(1,000) → Delta → IntegerPacking

See:

Change

Known Cartesian coordinate items request:

FixedPoint(1,000)
→ Delta
→ IntegerPacking
→ ByteArray

Expanded coordinate coverage

The project expanded the coordinate hint beyond _atom_site to known coordinate-bearing items from:

  • standard PDBx/mmCIF atom coordinates;
  • chemical-component model and ideal coordinates;
  • phasing-site categories;
  • solvent atom-site mapping;
  • ModelCIF template coordinates;
  • IHM starting-model coordinates;
  • IHM sphere, Gaussian, ensemble, and pseudo-site coordinates;
  • FLR/FPS mean-probe and atom-position coordinates.

This expanded item set is a py-mmcif project decision. It should not be presented as an exact copy of the Mol* field list.

Coordinate-factor experiment

Factors 1,000, 100,000, and 1,000,000 were evaluated during development.

The higher factors:

  • produced larger scaled integers;
  • reduced IntegerPacking efficiency;
  • increased BCIF size;
  • increased serialization time;
  • did not provide a meaningful practical accuracy benefit that justified those costs in the decoded comparisons.

Validation

The final factor-1,000 configuration passed:

  • all 37 Python round-trip validations with no value mismatches;
  • all 37 Java verifier checks;
  • Mol* parsing and rendering for conventional PDB and coordinate-heavy IHM examples.

The final benchmark showed particularly large reductions for coordinate-heavy files, but those file-level results reflect the complete implementation and should not be attributed only to the coordinate rule.

Result and decision

Factor 1,000 was retained because it:

  • aligns with the Mol* reference approach;
  • compresses more effectively than the larger tested factors;
  • avoids their additional serialization cost;
  • passed the complete interoperability and round-trip validation set.

Development evidence:

coordinate_factor_analysis_upload.zip

Final evidence:

03_round_trip/roundtrip_totals.txt
04_java_verifier/final_pr_encoding_verifier_summary.txt
05_molstar/molstar_4HHB.png
05_molstar/molstar_9A8K.png
08_final_benchmark/benchmark_summary.tsv

6. Add project-specific hints for anisotropic U values, IHM sphere radii, and repeated fields

The general four-chain classifier provides broad coverage, but development audits identified several high-volume fields with stable precision requirements or predictable value patterns.

These specialized decisions were developed from the project data. They are not claimed to be exact field rules copied from Mol*.

6.1 Six anisotropic-U fields

Problem

The six independent _atom_site_anisotrop.U[i][j] values commonly require four-decimal handling rather than the three-decimal coordinate policy.

Encoding them as direct floats leaves compression available, while using factor 1,000 could remove a decimal place that is intentionally represented in these columns.

Change

The following fields request factor 10,000:

_atom_site_anisotrop.U[1][1]
_atom_site_anisotrop.U[1][2]
_atom_site_anisotrop.U[1][3]
_atom_site_anisotrop.U[2][2]
_atom_site_anisotrop.U[2][3]
_atom_site_anisotrop.U[3][3]

Their requested chain is:

FixedPoint(10,000)
→ Delta
→ IntegerPacking
→ ByteArray
Validation

4BTS, which contains anisotropic-U data, passed:

  • the Python round-trip comparison;
  • the Java encoding verifier;
  • Mol* parsing and rendering.
Result and decision

Factor 10,000 was retained to preserve four-decimal U-matrix handling while allowing the values to use the integer compression pipeline.

Internal evidence:

03_round_trip/roundtrip_summary.tsv
04_java_verifier/final_pr_encoding_verifier.log
05_molstar/molstar_4BTS.png

6.2 _ihm_sphere_obj_site.object_radius

Problem

IHM sphere radii are numeric, often high-volume, and suitable for FixedPoint conversion, but their ordering does not necessarily make Delta useful and their values do not necessarily form long repeated runs.

Change

_ihm_sphere_obj_site.object_radius requests:

FixedPoint(1,000)
→ IntegerPacking
→ ByteArray

No fixed Delta or RunLength stage is requested.

Validation

The IHM validation corpus, including 9A8K and 9A25, passed the Python and Java checks. Representative IHM output also parsed and rendered in Mol*.

Result and decision

The direct FixedPoint-plus-IntegerPacking rule was retained as the explicit policy for this known IHM field.

Internal evidence:

03_round_trip/roundtrip_summary.tsv
04_java_verifier/final_pr_encoding_verifier.log
05_molstar/molstar_9A8K.png

6.3 Repeated high-volume float fields

Problem

Several large float columns frequently contain long repeated runs. Running the full four-candidate classifier on each known case adds classification work even when the expected useful pattern is already known.

Change

These four fields receive an automatically detected factor followed by a requested RunLength chain:

_atom_site.occupancy
_atom_site.B_iso_or_equiv
_ihm_sphere_obj_site.rmsf
_ihm_starting_model_coord.B_iso_or_equiv

The requested chain is:

FixedPoint(automatically detected factor)
→ RunLength
→ IntegerPacking
→ ByteArray

The factor is not hard-coded. It is determined from the actual values using the same bounded four-decimal factor detector as other general floats.

The existing RunLength guard prevents this hint from expanding the integer stream. When RunLength would produce more integers than the original array, that stage is omitted.

Validation

The final PDB and IHM corpus passed all round-trip and Java compatibility checks.

The aggregate benchmark validates the complete final configuration. It does not prove a separate percentage contribution for each of the four RunLength-hinted fields.

Result and decision

The hints were retained because they reflect observed repeated-value patterns, avoid running all four candidate chains for established high-volume cases, and retain the existing guard against harmful RunLength expansion.

Internal evidence:

01_final_source/BinaryCifWriter_final.py
03_round_trip/roundtrip_summary.tsv
04_java_verifier/final_pr_encoding_verifier.log
08_final_benchmark/benchmark_summary.tsv

7. Preserve existing mask behavior through the new float chains

Problem

Some CIF float columns contain special missing-value markers:

  • . means the value was not specified;
  • ? means the value is unknown.

These markers are not numbers, so they cannot be passed directly through numeric encoders such as FixedPoint, Delta, RunLength, or IntegerPacking.

External basis

BinaryCIF stores missing-value information in a separate mask:

0 = real value is present
1 = . not specified
2 = ? unknown

See:

Change

The existing py-mmcif mask behavior was preserved when the new float-encoding chains were added.

For example, this logical column:

1.25, ., 1.50, ?, 1.75

is handled as two separate arrays.

The numeric data stream becomes:

1.25, 0.0, 1.50, 0.0, 1.75

The mask becomes:

0, 1, 0, 2, 0

The temporary 0.0 values allow the numeric data to pass safely through the new compression chain.

The mask keeps the original meaning at each position:

  • data 0.0 with mask 1 is restored as .;
  • data 0.0 with mask 2 is restored as ?;
  • data 0.0 with mask 0 remains a real numeric zero.

When a mask is needed, it is:

  • stored as unsigned_integer_8;
  • encoded separately using RunLength → ByteArray;
  • written in the BinaryCIF column’s separate mask object.

When every value is present, no mask object is written.

Validation

The 37-structure round-trip validation reported:

Mask mismatches: 0

The same BCIF files also passed the Java encoding verifier, and representative files loaded successfully in Mol*.

Result and decision

The mask system itself was not redesigned by this change.

The existing separate-mask behavior was kept so that missing and unknown values remain correct while the numeric value stream can safely use the new FixedPoint and integer-encoding chains.

Internal evidence:

03_round_trip/roundtrip_summary.tsv
03_round_trip/roundtrip_totals.txt
04_java_verifier/final_pr_encoding_verifier_summary.txt

8. Keep unsafe and high-precision fallback columns numeric

Problem

Some float columns cannot safely use the bounded FixedPoint path because they:

  • require more than four detected decimal places;
  • overflow signed int32 after scaling;

An experimental alternative converted certain high-precision values to StringArrayMasked.

Although BinaryCIF supports StringArray, the experimental fallback encoded dictionary-defined float columns as strings. After decoding, downstream dictionary-aware code could therefore receive string values where numeric float values were expected, creating type mismatches and possible conversion or writing errors. Keeping the fallback disabled preserves the expected numeric representation.

Change

The final default is:

USE_STRING_FLOAT_FALLBACK = False

Therefore, unsupported general float columns remain numeric and use:

float ByteArray

The byte width continues to follow the writer’s existing useFloat64 setting.

This fallback also applies to unsafe specialized paths. A coordinate, anisotropic-U, sphere-radius, or repeated-field hint does not force FixedPoint when its values are non-finite or do not fit signed int32.

Validation

The focused final test confirmed that:

  • NaN uses ByteArray;
  • positive infinity uses ByteArray;
  • negative infinity uses ByteArray;
  • signed-int32 overflow uses ByteArray;
  • an ordinary coordinate still uses the intended FixedPoint chain.

The 37-structure round-trip audit found no raw-type mismatches, and all outputs passed the Java verifier.

Result and decision

The StringArray fallback remains in the implementation for possible future evaluation, but it is disabled in this PR.

The final writer keeps dictionary-defined float columns numeric because that is the lower-risk choice for type semantics and interoperability.

Internal evidence:

02_python_tests/focused_float_fallback_test_final.log
03_round_trip/roundtrip_raw_type_audit.tsv
03_round_trip/roundtrip_totals.txt
04_java_verifier/final_pr_encoding_verifier_summary.txt

9. Retain internal feature switches used during development

Problem

The implementation needed a controlled way to isolate the general FixedPoint path and each specialized rule during development.

Without separate switches, changing one rule at a time would require repeatedly editing the encoder logic and would make comparisons more error-prone.

Change

BinaryCifEncoders contains these internal class-level switches:

Switch Final default Behavior when True Behavior when False
USE_FIXED_POINT_FLOAT_ENCODING True Enables the new FixedPoint path and all enabled specialized rules Restores direct float ByteArray behavior for all float columns
USE_COORDINATE_CHAIN True Coordinates request factor 1,000 with Delta → IntegerPacking Coordinates retain factor 1,000 but use general four-chain selection
USE_ANISOTROP_U_CHAIN True Six U fields request factor 10,000 with Delta → IntegerPacking U fields use general factor detection and four-chain selection
USE_OBJECT_RADIUS_CHAIN True Sphere radius requests factor 1,000 with IntegerPacking Sphere radius uses the general float path
USE_RUN_LENGTH_FLOAT_HINTS True Four repeated fields request the RunLength chain Those fields use general four-chain selection
USE_STRING_FLOAT_FALLBACK False Eligible unsupported floats may use StringArrayMasked Unsupported floats remain numeric ByteArray data

Except for the master FixedPoint switch, disabling one specialized switch does not force that field directly to ByteArray. It sends the field through the general FixedPoint logic instead.

Validation scope

The final validation package tests the production defaults shown above.

Result and decision

The switches remain internal class attributes because they make the individual policies reviewable and allow for quick troubleshooting and experimenting.

Internal evidence:

01_final_source/BinaryCifWriter_final.py

Before-and-after summary

Column or condition Production behavior Final behavior
Known Cartesian coordinates Direct float ByteArray Factor 1,000 with requested Delta → IntegerPacking → ByteArray
Six anisotropic-U fields Direct float ByteArray Factor 10,000 with requested Delta → IntegerPacking → ByteArray
_ihm_sphere_obj_site.object_radius Direct float ByteArray Factor 1,000 with IntegerPacking → ByteArray
Four repeated high-volume fields Direct float ByteArray Automatically detected factor with requested RunLength → IntegerPacking → ByteArray
Other safe floats requiring up to four decimal places Direct float ByteArray Smallest payload from four FixedPoint integer-chain candidates
General floats requiring more than four detected decimal places Float ByteArray Float ByteArray
Non-finite values Float ByteArray Float ByteArray
Unsafe signed-int32 scaling Float ByteArray Float ByteArray
Missing and unknown values Separate mask Separate mask; unchanged
Dictionary-typed integer columns Existing integer path Unchanged
Dictionary-typed string columns Existing StringArray path Unchanged

The final result is a deliberate size-versus-write-time tradeoff:

71.153% smaller total BCIF output
35.031% slower aggregate median serialization time

Controlled benchmark

Methodology

  • Python: 3.13.0
  • Platform: WSL2 Linux, x86-64
  • Writer setting: useFloat64=True
  • String fallback: disabled
  • Structures: 10 spanning PDB, IHM, AlphaFold, and ModelCIF
  • Warm-up: 1 run per structure per writer
  • Measured repetitions: 5 per structure per writer
  • Primary timing statistic: median
  • Timed operation: BinaryCifWriter.serialize(), including output-file writing
  • Excluded from timing: input CIF parsing and dictionary loading
  • Bias reduction: baseline/final execution order alternated between measured runs

Aggregate result

Metric Production writer Final writer Change
Total BCIF size 167,044,307 bytes 48,187,201 bytes 71.153% smaller
Sum of per-structure median serialization times 45.756497 s 61.785365 s 35.031% slower

Additional aggregate statistics:

  • total bytes saved: 118,857,106;
  • median per-structure size reduction: 62.001%;
  • median per-structure serialization change: 39.381% slower.

Per-structure results

Structure Type / coverage Production BCIF Final BCIF Size change Production median Final median Time change
2BVK PDB, small 88,586 86,997 1.794% smaller 0.009673 s 0.011364 s 17.488% slower
4HHB PDB, small/medium 530,943 312,728 41.100% smaller 0.088222 s 0.136012 s 54.170% slower
12GB PDB, medium 4,273,820 1,289,137 69.836% smaller 0.929353 s 1.303115 s 40.218% slower
4BTS PDB, large, anisotropic U 19,845,300 6,442,338 67.537% smaller 4.200834 s 6.457648 s 53.723% slower
3J3Q PDB, very large 113,322,354 29,156,671 74.271% smaller 28.511336 s 39.500949 s 38.545% slower
9A01 IHM, small 1,536,585 486,643 68.330% smaller 0.260257 s 0.366516 s 40.828% slower
9A25 IHM, large 9,965,128 6,245,536 37.326% smaller 9.586133 s 10.056246 s 4.904% slower
9A8K IHM, coordinate-heavy 16,699,656 3,806,185 77.208% smaller 2.009673 s 3.740031 s 86.101% slower
AF_AFA0A017SEY2F1 AlphaFold/CSM 196,016 105,881 45.983% smaller 0.034709 s 0.046315 s 33.439% slower
MA_MABAKCEPC0001 ModelCIF/CSM 585,919 255,085 56.464% smaller 0.126307 s 0.167168 s 32.350% slower

Raw benchmark artifacts generated during validation:

  • benchmark_runs.tsv;
  • benchmark_summary.tsv;
  • benchmark_aggregate.tsv;
  • benchmark_metadata.txt.

Interpretation

The optimization strongly favors reduced output size over write speed:

  • large and coordinate-heavy structures show the largest compression gains;
  • 9A8K was 77.208% smaller, while serialization time increased by 86.101%;
  • 3J3Q saved more than 84 million bytes and was 74.271% smaller;
  • 9A25 achieved a 37.326% size reduction with only a 4.904% timing increase;
  • very small files such as 2BVK have little redundant float data, so the additional classification work provides limited size benefit.

The aggregate result should be used for the overall tradeoff. A simple arithmetic average of individual percentages can overstate the effect of small files and is not used as the primary result.

@hriday1136 hriday1136 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added suggestions to be considered. No major changes at all.

return 10 ** mantissaDigits

# Try each FixedPoint integer chain and keep the smallest byte output
def __encodeBestFixedPointChain(self, colDataList, factor):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might want to see whether it is better to check all four encoding chains for each column or hardcoding one specific chain for all columns. Can be checked in terms of size and runtime tradeoff.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The four-chain search applies only to non-hardcoded float columns with four or fewer decimal places, so although it covers many columns, those columns contain a relatively small share of the total values.

Comment thread mmcif/io/BinaryCifWriter.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves BinaryCIF serialization for float-typed columns by introducing FixedPoint encoding with integer compression chains, using column-aware hints for high-volume structural attributes (e.g., coordinates, anisotropic U), and a general “choose smallest” chain selection for other float columns, while preserving existing mask semantics.

Changes:

  • Pass (catName, atName) through the internal encoding pipeline so float encoders can apply item-specific policies.
  • Add FixedPoint encoder support and propagate the working data type through chained encoders so downstream ByteArray metadata matches the transformed integer data.
  • Implement float-chain selection logic (special-case hints + general factor detection + best-of-four integer chains), with ByteArray fallback paths for unsafe data.

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

Comment thread mmcif/io/BinaryCifWriter.py
Comment thread mmcif/io/BinaryCifWriter.py Outdated
Comment thread mmcif/io/BinaryCifWriter.py Outdated
Comment thread mmcif/io/BinaryCifWriter.py Outdated
Comment thread mmcif/io/BinaryCifWriter.py Outdated
yusufm99 added 2 commits June 28, 2026 17:00
Updated comments for clarity and adjusted descriptions of encoding options.
@epeisach

Copy link
Copy Markdown
Collaborator

So I ran the tox tests and picked up on some style issues:

mmcif/io/BinaryCifWriter.py:208:5: E303 too many blank lines (2)
mmcif/io/BinaryCifWriter.py:215:5: E303 too many blank lines (2)
mmcif/io/BinaryCifWriter.py:228:5: E303 too many blank lines (2)
mmcif/io/BinaryCifWriter.py:234:5: E303 too many blank lines (2)
mmcif/io/BinaryCifWriter.py:240:5: E303 too many blank lines (2)
mmcif/io/BinaryCifWriter.py:248:5: E303 too many blank lines (2)
mmcif/io/BinaryCifWriter.py:256:5: E303 too many blank lines (2)
mmcif/io/BinaryCifWriter.py:695:5: E303 too many blank lines (2)
mmcif/io/BinaryCifWriter.py:696:5: E301 expected 1 blank line, found 0
mmcif/io/BinaryCifWriter.py:754:5: E303 too many blank lines (5)
mmcif/io/BinaryCifWriter.py:898:5: E303 too many blank lines (2)

lint_pylint-py39: commands[1]> pylint --disable=R,C --reports=n --rcfile=/private/tmp/py-mmcif/pylintrc mmcif/
************* Module mmcif.io.BinaryCifWriter
mmcif/io/BinaryCifWriter.py:740:42: E1136: Value 'best' is unsubscriptable (unsubscriptable-object)

For pylint issue - it is a false report... I would add at the end of the line " # pylint: disable E1136"
The code is correct.

Comment thread mmcif/io/BinaryCifWriter.py
def encodeWithMask(self, colDataList, colMaskList, encodingType, catName=None, atName=None):
"""Encode the data using the input mask and encoding type returning encoded data and encoding instructions.

Args:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring should be updated for new arguments

@epeisach epeisach left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have provided comments inline with code... The largest change involves your hints - should use an Enum, and then two dictionaries - enabled, and configuration.


# True: enable the new FixedPoint-based float encoding logic and all enabled
# specialized paths below.
# False: bypass every float optimization below and encode all floats directly

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these switches accessible from outside the class? They should be private. If you want to add an interface to switch on or off internal flags -- create it. Abstraction barriers matter.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Ezra here—they should at minimum be private, i.e.,:

    _USE_COORDINATE_CHAIN = True
    _USE_ANISOTROP_U_CHAIN = True
    ...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, Ezra and I talked about this and came up with a decision on what to do here. I'll share in my parent review comment.


if encType == "ByteArray":
colDataList, encDict = self.byteArrayEncoderTyped(colDataList, dataType)
colDataList, encDict = self.byteArrayEncoderTyped(colDataList, currentDataType)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment explaining why currentDataType is changed from float to integer for the byte packing code...

Comment thread mmcif/io/BinaryCifWriter.py
Comment thread mmcif/io/BinaryCifWriter.py

def __encodeColumnData(self, colDataList, dataType):
# Accept category/item names so encoder selection can use column-specific hints
def __encodeColumnData(self, colDataList, dataType, catName=None, atName=None):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have a default ofr catName or atName. __encodeColumnData is invoked only one place in the code.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it's only used once in the code, I think having non-positional parameters with defaults is better practice, since it avoids having to worry about the order in which arguments are passed later.

Comment thread mmcif/io/BinaryCifWriter.py
srcType = colTypedDataList.dtype or ("float_64" if self.__useFloat64 else "float_32")
encodedColDataList = [self.__roundLikeMolStar(float(v) * factor) for v in colTypedDataList.data]

if not self.__fitsInt32(encodedColDataList):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear what to do here... Exception is the best right now - as I have no idea how to rollback and encode as float_32/float_64....

if catName is None or atName is None:
return ""

cat = str(catName)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you envision that catName will not be a string?

Comment thread mmcif/io/BinaryCifWriter.py
Comment thread mmcif/io/BinaryCifWriter.py

@piehld piehld left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @yusufm99, this is fantastic work!

I've left a couple comments and questions for you throughout the code, as well as resolved some of Ezra's comments after having discussed some ideas with him, so when you review our comments please do so on GitHub (as that will hide any comments that have already been marked as resolved).

The main change @epeisach and I discussed is to move all the "switches" and other pre-defined factors to a separate configuration file config.py, and merge the different data item lists together such that you don't need so many "if/else" checks to use them. To give you an idea of what it should look like, I'll send you a draft of the file by email. You would then import the config class into the BinaryCifWriter.py file and access the different config options from the config object.

Another change that Ezra and I thought would be worth including is to encode all floats with ≤ 2 rows as strings (i.e., if there are only two rows in a particular float column, encode it as a string rather than a float). This should be controlled by another configuration setting (e.g., STRING_FALLBACK_MAX_FLOAT_ROWS). However, before you proceed with implementing that, I'm wondering if you think it'd make more sense to handle this in @hriday1136's code? Would that be a better place to determine if the type should be "string" or "float" based on the number of rows?

Comment on lines 6 to +11
#
# Float encoding updates:
# - FixedPoint support with IntegerPacking, RunLength, and Delta chains.
# - Column-specific chains for known high-volume float attributes.
# - Automatic selection of the smallest general FixedPoint chain.
# - Optional StringArray fallback for high-precision floats (disabled by default).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the top-level "Updates" comment there, and add yours as a specific update (dating ahead to 10-Jul-2026)

Suggested change
#
# Float encoding updates:
# - FixedPoint support with IntegerPacking, RunLength, and Delta chains.
# - Column-specific chains for known high-volume float attributes.
# - Automatic selection of the smallest general FixedPoint chain.
# - Optional StringArray fallback for high-precision floats (disabled by default).
# Updates:
# 10-Jul-2026 ym Float encoding updates:
# - FixedPoint support with IntegerPacking, RunLength, and Delta chains.
# - Column-specific chains for known high-volume float attributes.
# - Automatic selection of the smallest general FixedPoint chain.
# - Optional StringArray fallback for high-precision floats (disabled by default).


def __encodeColumnData(self, colDataList, dataType):
# Accept category/item names so encoder selection can use column-specific hints
def __encodeColumnData(self, colDataList, dataType, catName=None, atName=None):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it's only used once in the code, I think having non-positional parameters with defaults is better practice, since it avoids having to worry about the order in which arguments are passed later.

# specialized paths below.
# False: bypass every float optimization below and encode all floats directly
# with the original float ByteArray behavior.
USE_FIXED_POINT_FLOAT_ENCODING = True

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think by default we always want this (unless useStringTypes=True), right @epeisach? So, I would suggest this switch be removed.


# True: enable the new FixedPoint-based float encoding logic and all enabled
# specialized paths below.
# False: bypass every float optimization below and encode all floats directly

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Ezra here—they should at minimum be private, i.e.,:

    _USE_COORDINATE_CHAIN = True
    _USE_ANISOTROP_U_CHAIN = True
    ...

Comment thread mmcif/io/BinaryCifWriter.py

# True: enable the new FixedPoint-based float encoding logic and all enabled
# specialized paths below.
# False: bypass every float optimization below and encode all floats directly

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, Ezra and I talked about this and came up with a decision on what to do here. I'll share in my parent review comment.

Comment thread mmcif/io/BinaryCifWriter.py
Comment thread mmcif/io/BinaryCifWriter.py
Comment on lines +749 to +753





Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave only one blank line between adjacent methods.

Comment thread mmcif/io/BinaryCifWriter.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants