fix(metadata): restore TlsGroup tensor initialization#428
Open
CV-GPhL wants to merge 1 commit into
Open
Conversation
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.
This was referenced May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restores NAN initialization of
TlsGroup::T,::L, and::Sthat was removed during the Doxygen documentation PR series (#414).The bug
The original code initialized these tensors to NAN:
After #414 these became:
Why it matters
The mmCIF writer uses
number_or_qmarkinsrc/to_mmcif.cpp:With the original NAN initializers, a
TlsGroupthat was instantiated but never populated would emit?(unset) for all tensor components in mmCIF output. Without the initializers:SMat33<double> T, Lhold undefined values → garbage numbers in outputMat33 Sdefaults to identity matrix → emitted as1.0 0.0 0.0 0.0 1.0 0.0 0.0 0.0 1.0, indistinguishable from real TLS S-tensor dataA 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_bbut missed theTlsGrouptensors. This PR completes that fix.Companion PR
Related: #427 fixes a separate missed initialization (
GridMeta::spacegroup) from the same PR series.