Skip to content

fix(metadata): restore TlsGroup tensor initialization#428

Open
CV-GPhL wants to merge 1 commit into
project-gemmi:masterfrom
CV-GPhL:fix/restore-tlsgroup-initializers
Open

fix(metadata): restore TlsGroup tensor initialization#428
CV-GPhL wants to merge 1 commit into
project-gemmi:masterfrom
CV-GPhL:fix/restore-tlsgroup-initializers

Conversation

@CV-GPhL
Copy link
Copy Markdown
Contributor

@CV-GPhL CV-GPhL commented May 22, 2026

Restores NAN initialization of TlsGroup::T, ::L, and ::S that was removed during the Doxygen documentation PR series (#414).

The bug

The original code initialized these tensors to NAN:

SMat33<double> T = {NAN, NAN, NAN, NAN, NAN, NAN};
SMat33<double> L = {NAN, NAN, NAN, NAN, NAN, NAN};
Mat33 S = Mat33{NAN};

After #414 these became:

SMat33<double> T;
SMat33<double> L;
Mat33 S;

Why it matters

The mmCIF writer uses number_or_qmark in src/to_mmcif.cpp:

inline std::string number_or_qmark(double d) {
  return std::isnan(d) ? "?" : to_str(d);
}

With the original NAN initializers, a TlsGroup that was instantiated but never populated would emit ? (unset) for all tensor components in mmCIF output. Without the initializers:

  • SMat33<double> T, L hold undefined values → garbage numbers in output
  • Mat33 S defaults to identity matrix → emitted as 1.0 0.0 0.0 0.0 1.0 0.0 0.0 0.0 1.0, indistinguishable from real TLS S-tensor data

A partially populated TlsGroup (e.g. origin and selections set, tensors not yet computed) would silently write misleading numerical data to disk that downstream refinement programs would treat as valid.

Relation to earlier fix

Commit 2c1b0c5 restored the analogous initialization for RefinementInfo::aniso_b but missed the TlsGroup tensors. This PR completes that fix.

Companion PR

Related: #427 fixes a separate missed initialization (GridMeta::spacegroup) from the same PR series.

Restores NAN initialization of TlsGroup::T, ::L, and ::S that was
removed during the Doxygen documentation PR series (project-gemmi#414).

The original code initialized these tensors to NAN so that the mmCIF
writer (number_or_qmark in src/to_mmcif.cpp) would emit "?" (unset)
for empty TLS groups. With the initializers removed:

- SMat33<double> T, L hold undefined values
- Mat33 S defaults to identity matrix (1,0,0;0,1,0;0,0,1)

A TlsGroup that is instantiated but only partially populated (e.g.,
origin and selections set, tensors not yet computed) would now write
garbage numbers for T/L and a misleading identity matrix for S into
mmCIF output, where downstream refinement programs would read them
as valid tensor data.

The earlier fix commit 2c1b0c5 restored the analogous initialization
in RefinementInfo::aniso_b but missed the TlsGroup tensors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant