Merge branch 'mv-ncs' into rc175: models expanded by their strict NCS operators; the written cell follows the model's indexing

Exact: p.hkl md5 and MTZ reflection data identical on 18 sets. 4nwv R-free 0.545 -> 0.297 and cell in the depositor's
frame; written cells corrected on 9rcs, 9b22, 9qw8, myob_x06da_powder_1.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EizBKhTqrAYago9KJG3dA3

common/UnitCell.h

@@ -17,6 +17,7 @@ struct UnitCell {
     float gamma;
 
     operator gemmi::UnitCell() const;
+    bool operator==(const UnitCell &) const = default;
 
     [[nodiscard]] bool is_finite() const;
     [[nodiscard]] bool is_close(const UnitCell &ref, float dist_tolerance, float angle_tolerance_deg) const;

gemmi_gph/CMakeLists.txt

@@ -9,11 +9,13 @@
 #
 # to_mmcif/to_pdb are the writing side, which `rugnux --model` needs to put the model it has placed
 # against the data on disk. to_mmcif.cpp calls use_hetatm() out of to_pdb.cpp, so the two come together.
+#
+# assembly.cpp expands a deposited model's strict-NCS operators into the full asymmetric unit.
 ADD_LIBRARY(gemmi STATIC
         symmetry.cpp gz.cpp mtz.cpp sprintf.cpp xds_ascii.cpp
         pdb.cpp resinfo.cpp polyheur.cpp calculate.cpp eig3.cpp ccp4.cpp
         read_cif.cpp mmcif.cpp json.cpp mmread_gz.cpp
-        to_mmcif.cpp to_pdb.cpp
+        to_mmcif.cpp to_pdb.cpp assembly.cpp
         gemmi/cellred.hpp
         gemmi/symmetry.hpp
         gemmi/fail.hpp

gemmi_gph/assembly.cpp

@@ -0,0 +1,466 @@
+// Copyright 2020 Global Phasing Ltd.
+
+#include "gemmi/assembly.hpp"
+
+#include <memory>             // unique_ptr
+#include "gemmi/modify.hpp"   // transform_pos_and_adp
+#include "gemmi/neighbor.hpp" // NeighborSearch
+
+namespace gemmi {
+
+namespace {
+
+bool any_subchain_matches(const Chain& chain, const Assembly::Gen& gen) {
+  const std::string* prev_subchain = nullptr;
+  for (const Residue& res : chain.residues)
+    if (prev_subchain == nullptr || res.subchain != *prev_subchain) {
+      if (in_vector(res.subchain, gen.subchains))
+        return true;
+      prev_subchain = &res.subchain;
+    }
+  return false;
+}
+
+// chain name mappings old->new for each Assembly::Operator used
+struct ChainMap {
+  bool uses_segments = false;
+  std::string id;
+  std::map<std::string, std::string> names;
+};
+struct AssemblyMapping {
+  HowToNameCopiedChain how;
+  // records subchain name correspondence (new->old), used for updating Entity::subchain
+  std::map<std::string, std::string> sub;
+  std::vector<ChainMap> chain_maps;
+};
+
+void update_address(AtomAddress& a, const ChainMap& chain_map) {
+  auto it = chain_map.names.find(a.chain_name);
+  if (it != chain_map.names.end())
+    a.chain_name = it->second;
+}
+
+void remove_cras(Model& model, std::vector<CRA>& vec) {
+  // sort in reverse order, so items can be erased without invalidating pointers
+  std::sort(vec.begin(), vec.end(), [](const CRA& a, const CRA& b) {
+      return std::tie(a.chain, a.residue, a.atom) > std::tie(b.chain, b.residue, b.atom);
+  });
+  const Atom* prev_a = nullptr;
+  for (CRA& cra : vec) {
+    if (cra.atom == prev_a)
+      continue;
+    prev_a = cra.atom;
+    auto atom_idx = cra.atom - cra.residue->atoms.data();
+    cra.residue->atoms.erase(cra.residue->atoms.begin() + atom_idx);
+    if (cra.residue->atoms.empty()) {
+      auto res_idx = cra.residue - cra.chain->residues.data();
+      cra.chain->residues.erase(cra.chain->residues.begin() + res_idx);
+      if (cra.chain->residues.empty()) {
+        auto chain_idx = cra.chain - model.chains.data();
+        model.chains.erase(model.chains.begin() + chain_idx);
+      }
+    }
+  }
+}
+
+Model make_assembly_(const Assembly& assembly, const Model& model,
+                     HowToNameCopiedChain how, const Logger& logger,
+                     AssemblyMapping* mapping) {
+  Model new_model(model.num);
+  ChainNameGenerator namegen(how);
+  std::map<std::string, std::string> subs = model.subchain_to_chain();
+  int counter = 0;
+  for (const Assembly::Gen& gen : assembly.generators) {
+    bool all_chains = (!gen.chains.empty() && gen.chains[0] == "(all)");
+    for (const Assembly::Operator& oper : gen.operators) {
+      if (logger.callback) {
+        std::string note = cat("Applying ", oper.name, " to");
+        if (all_chains)
+          note += " all chains";
+        else if (!gen.chains.empty())
+          cat_to(note, " chains: ", join_str(gen.chains, ','));
+        else if (!gen.subchains.empty())
+          cat_to(note, " subchains: ", join_str(gen.subchains, ','));
+        logger.note(note);
+      }
+      if (!all_chains) {
+        for (const std::string& chain_name : gen.chains)
+          if (!model.find_chain(chain_name))
+            logger.err("no chain ", chain_name);
+        for (const std::string& subchain_name : gen.subchains)
+          if (subs.find(subchain_name) == subs.end())
+            logger.err("no subchain ", subchain_name);
+      }
+      // chains are not merged here, multiple chains may have the same name
+      ChainMap chain_map;
+      if (counter != 0) {
+        chain_map.uses_segments = (how == HowToNameCopiedChain::Dup);
+        chain_map.id = std::to_string(counter);
+      }
+      for (const Chain& chain : model.chains) {
+        // PDB files specify bioassemblies in terms of chains,
+        // mmCIF files in terms of subchains.
+        bool whole_chain = (all_chains || in_vector(chain.name, gen.chains));
+        if (whole_chain ||
+            (!gen.subchains.empty() && any_subchain_matches(chain, gen))) {
+          // try add a new empty chain, but first figure out the name for it
+          auto result = chain_map.names.emplace(chain.name, "");
+          if (result.second)  // insertion happened - generate a new chain name
+            result.first->second = namegen.make_new_name(chain.name, counter+1);
+          new_model.chains.emplace_back(result.first->second);
+          Chain& new_chain = new_model.chains.back();
+          // add residues to the chain
+          for (const Residue& res : chain.residues)
+            if (whole_chain || in_vector(res.subchain, gen.subchains)) {
+              new_chain.residues.push_back(res);
+              Residue& new_res = new_chain.residues.back();
+              transform_pos_and_adp(new_res, oper.transform);
+              if (!new_res.subchain.empty()) {
+                // change subchain name for the residue
+                if (how == HowToNameCopiedChain::Short)
+                  new_res.subchain = new_chain.name + ":" + new_res.subchain;
+                else if (how == HowToNameCopiedChain::AddNumber)
+                  new_res.subchain += new_chain.name.substr(chain.name.size());
+                if (mapping)
+                  mapping->sub.emplace(new_res.subchain, res.subchain);
+              }
+              if (chain_map.uses_segments)
+                new_res.segment = chain_map.id;
+            }
+        }
+      }
+      if (mapping)
+        mapping->chain_maps.push_back(std::move(chain_map));
+      ++counter;
+    }
+  }
+  return new_model;
+}
+
+
+void expand_ncs_model_(Model& model, const std::vector<NcsOp>& ncs,
+                       HowToNameCopiedChain how, AssemblyMapping* mapping) {
+  // For HowToNameCopiedChain::Dup we used to set segment="0" for original
+  // residues. Now it's left blank. Not sure which is better.
+  size_t orig_size = model.chains.size();
+  ChainNameGenerator namegen(model, how);
+  if (mapping) {  // add identities to AssemblyMapping
+    mapping->chain_maps.emplace_back();
+    ChainMap& chain_map = mapping->chain_maps.back();
+    // chain_map.id is left empty
+    for (const Chain& chain : model.chains) {
+      chain_map.names.emplace(chain.name, chain.name);
+      for (const ConstResidueSpan& span : chain.subchains()) {
+        const std::string& sub_id = span.subchain_id();
+        mapping->sub.emplace(sub_id, sub_id);
+      }
+    }
+  }
+  int counter = 0;
+  for (const NcsOp& op : ncs)
+    if (!op.given) {
+      ChainMap chain_map;
+      chain_map.uses_segments = (how == HowToNameCopiedChain::Dup);
+      chain_map.id = op.id;
+      ++counter;
+      for (size_t i = 0; i != orig_size; ++i) {
+        model.chains.push_back(model.chains[i]);
+        Chain& new_chain = model.chains.back();
+        const std::string& old_name = model.chains[i].name;
+        auto result = chain_map.names.emplace(old_name, "");
+        if (how != HowToNameCopiedChain::Dup) {
+          if (result.second) // if insertion happened - generate a new chain name
+            result.first->second = namegen.make_new_name(old_name, counter);
+          new_chain.name = result.first->second;
+        }
+        for (Residue& new_res : new_chain.residues) {
+          transform_pos_and_adp(new_res, op.tr);
+          if (!new_res.subchain.empty()) {
+            std::string old_subchain = new_res.subchain;
+            new_res.subchain = new_chain.name + ":" + new_res.subchain;
+            if (mapping)
+              mapping->sub.emplace(new_res.subchain, old_subchain);
+          }
+          if (chain_map.uses_segments)
+            new_res.segment = chain_map.id;
+        }
+      }
+      if (mapping)
+        mapping->chain_maps.push_back(std::move(chain_map));
+    }
+}
+
+
+void finalize_expansion(Structure& st, const AssemblyMapping& mapping,
+                        double merge_dist, bool expanding_ncs) {
+  if (merge_dist > 0)
+    for (Model& model : st.models) {
+      merge_atoms_in_expanded_model(model, gemmi::UnitCell(), merge_dist);
+      assign_serial_numbers(model);
+    }
+
+  // update Entity::subchains
+  if (!mapping.sub.empty())
+    for (Entity& ent : st.entities) {
+      std::vector<std::string> new_subchains;
+      for (const std::string& s : ent.subchains)
+        for (const auto& new_old : mapping.sub)
+          if (new_old.second == s)
+            new_subchains.push_back(new_old.first);
+      ent.subchains = std::move(new_subchains);
+    }
+
+  // connections
+  std::vector<Connection> new_connections;
+  for (const Connection& conn : st.connections)
+    if (expanding_ncs || conn.asu == Asu::Same) {
+      bool first = true;
+      for (const ChainMap& chain_map : mapping.chain_maps) {
+        auto ch1 = chain_map.names.find(conn.partner1.chain_name);
+        auto ch2 = chain_map.names.find(conn.partner2.chain_name);
+        if (ch1 != chain_map.names.end() &&
+            ch2 != chain_map.names.end()) {
+          Connection new_conn = conn;
+          new_conn.partner1.chain_name = ch1->second;
+          new_conn.partner2.chain_name = ch2->second;
+          if (chain_map.uses_segments)
+            new_conn.partner1.res_id.segment =
+            new_conn.partner2.res_id.segment = chain_map.id;
+          if (st.models[0].find_atom(new_conn.partner1) &&
+              st.models[0].find_atom(new_conn.partner2)) {
+            if (!first) {
+              cat_to(new_conn.name, '.', chain_map.id);
+              first = false;
+            }
+            new_connections.push_back(new_conn);
+          }
+        }
+      }
+    } else {
+      // connections other than 1_555 are lost when making assembly
+      // if it's needed - get in touch
+    }
+  st.connections = std::move(new_connections);
+
+  if (mapping.how == HowToNameCopiedChain::Dup)
+    return;
+
+  // cispeps
+  std::vector<CisPep> new_cispeps;
+  new_cispeps.reserve(st.cispeps.size() * mapping.chain_maps.size());
+  for (const CisPep& cispep : st.cispeps)
+    for (const ChainMap& chain_map : mapping.chain_maps) {
+      new_cispeps.push_back(cispep);
+      update_address(new_cispeps.back().partner_c, chain_map);
+      update_address(new_cispeps.back().partner_n, chain_map);
+    }
+  st.cispeps = std::move(new_cispeps);
+
+  // secondary structure - helices
+  std::vector<Helix> new_helices;
+  new_helices.reserve(st.helices.size() * mapping.chain_maps.size());
+  for (const Helix& helix : st.helices)
+    for (const ChainMap& chain_map : mapping.chain_maps) {
+      new_helices.push_back(helix);
+      update_address(new_helices.back().start, chain_map);
+      update_address(new_helices.back().end, chain_map);
+    }
+  st.helices = std::move(new_helices);
+
+  // secondary structure - sheets
+  std::vector<Sheet> new_sheets;
+  new_sheets.reserve(st.sheets.size() * mapping.chain_maps.size());
+  for (const Sheet& sheet : st.sheets)
+    for (const ChainMap& chain_map : mapping.chain_maps) {
+      new_sheets.push_back(sheet);
+      for (Sheet::Strand& strand : new_sheets.back().strands) {
+        update_address(strand.start, chain_map);
+        update_address(strand.end, chain_map);
+        update_address(strand.hbond_atom2, chain_map);
+        update_address(strand.hbond_atom1, chain_map);
+      }
+    }
+  st.sheets = new_sheets;
+}
+
+} // anonymous namespace
+
+Model make_assembly(const Assembly& assembly, const Model& model,
+                    HowToNameCopiedChain how, const Logger& logging) {
+  return make_assembly_(assembly, model, how, logging, nullptr);
+}
+
+void transform_to_assembly(Structure& st, const std::string& assembly_name,
+                           HowToNameCopiedChain how, const Logger& logging,
+                           bool keep_spacegroup, double merge_dist) {
+  const Assembly* assembly = st.find_assembly(assembly_name);
+  std::unique_ptr<Assembly> p1_assembly;
+  if (!assembly) {
+    if (assembly_name == "unit_cell") {
+      p1_assembly.reset(new Assembly(pseudo_assembly_for_unit_cell(st.cell)));
+      assembly = p1_assembly.get();
+    } else if (st.assemblies.empty()) {
+      fail("no bioassemblies are listed for this structure");
+    } else {
+      fail("wrong assembly name, use one of: " +
+           join_str(st.assemblies, ' ', [](const Assembly& a) { return a.name; }));
+    }
+  }
+
+  AssemblyMapping mapping;
+  mapping.how = how;
+  AssemblyMapping* mapping_ptr = &mapping;
+  for (Model& model : st.models) {
+    model = make_assembly_(*assembly, model, how, logging, mapping_ptr);
+    mapping_ptr = nullptr;  // AssemblyMapping is based only on the first model
+  }
+  finalize_expansion(st, mapping, merge_dist, false);
+
+  // Should Assembly instructions be kept or removed? Currently - removing.
+  st.assemblies.clear();
+
+  if (!keep_spacegroup) {
+    st.spacegroup_hm = "P 1";  // maybe it should be empty?
+    if (assembly_name != "unit_cell")
+      st.cell = UnitCell();
+  } else {
+    st.cell.images.clear();
+  }
+}
+
+Model expand_ncs_model(const Model& model, const std::vector<NcsOp>& ncs,
+                       HowToNameCopiedChain how) {
+  Model model_copy = model;
+  expand_ncs_model_(model_copy, ncs, how, nullptr);
+  return model_copy;
+}
+
+void expand_ncs(Structure& st, HowToNameCopiedChain how, double merge_dist) {
+  AssemblyMapping mapping;
+  mapping.how = how;
+  AssemblyMapping* mapping_ptr = &mapping;
+  for (Model& model : st.models) {
+    expand_ncs_model_(model, st.ncs, how, mapping_ptr);
+    mapping_ptr = nullptr;  // AssemblyMapping is based only on the first model
+  }
+  finalize_expansion(st, mapping, merge_dist, true);
+
+  for (NcsOp& op : st.ncs)
+    op.given = true;
+
+  st.setup_cell_images();
+}
+
+
+void merge_atoms_in_expanded_model(Model& model, const UnitCell& cell, double max_dist,
+                                   bool compare_serial) {
+  using Mark = NeighborSearch::Mark;
+  NeighborSearch ns(model, cell, 4.0);
+  ns.populate(true);
+  std::vector<CRA> to_be_deleted;
+  for (int n_ch = 0; n_ch != (int) model.chains.size(); ++n_ch) {
+    Chain& chain = model.chains[n_ch];
+    for (int n_res = 0; n_res != (int) chain.residues.size(); ++n_res) {
+      Residue& res = chain.residues[n_res];
+      for (int n_atom = 0; n_atom != (int) res.atoms.size(); ++n_atom) {
+        Atom& atom = res.atoms[n_atom];
+        std::vector<std::pair<CRA, int>> equiv;
+        ns.for_each_cell(atom.pos, [&](std::vector<Mark>& marks, const Fractional& fr) {
+            for (Mark& m : marks) {
+              // We look for the same atoms, but copied to a different chain.
+              // First quick check that filters out most of non-matching pairs.
+              if (m.altloc != atom.altloc || m.element != atom.element ||
+                  m.chain_idx == n_ch || m.atom_idx != n_atom)
+                continue;
+              // Now check if everything else matches.
+              CRA cra = m.to_cra(model);
+              if (cra.atom &&
+                  (!compare_serial || cra.atom->serial == atom.serial) &&
+                  cra.atom->name == atom.name &&
+                  cra.atom->b_iso == atom.b_iso &&
+                  cra.residue->matches_noseg(res) &&
+                  m.pos.dist_sq(ns.grid.unit_cell.orthogonalize(fr)) < sq(max_dist))
+                equiv.emplace_back(cra, m.image_idx);
+            }
+        });
+        if (!equiv.empty()) {
+          Position pos_sum = atom.pos;
+          for (auto& t : equiv) {
+            CRA& cra = t.first;
+            pos_sum += ns.grid.unit_cell.find_nearest_pbc_position(
+                                          atom.pos, cra.atom->pos, t.second);
+            // The atoms in equiv are to be discarded later.
+            // Deleting now would invalidate indices in NeighborSearch.
+            to_be_deleted.push_back(cra);
+            // Modify the atoms to avoid processing them again.
+            cra.atom->serial = -1;
+            cra.atom->name.clear();
+          }
+          size_t n = 1 + equiv.size();
+          atom.pos = pos_sum / double(n);
+          atom.occ = std::min(1.f, n * atom.occ);
+        }
+      }
+    }
+  }
+  remove_cras(model, to_be_deleted);
+}
+
+
+void shorten_chain_names(Structure& st) {
+  ChainNameGenerator namegen(HowToNameCopiedChain::Short);
+  Model& model0 = st.models[0];
+  size_t max_len = model0.chains.size() < 63 ? 1 : 2;
+  for (const Chain& chain : model0.chains)
+    if (chain.name.length() <= max_len)
+      namegen.used_names.push_back(chain.name);
+  for (Chain& chain : model0.chains)
+    if (chain.name.length() > max_len)
+      rename_chain(st, chain.name,
+                   namegen.make_short_name(chain.name.substr(0, max_len)));
+}
+
+
+static std::vector<Chain> split_chain_by_segments(Chain& orig, ChainNameGenerator& namegen) {
+  std::vector<Chain> chains;
+  std::vector<Residue> orig_res;
+  orig_res.swap(orig.residues);
+  int n = 0;
+  for (auto start = orig_res.begin(); start != orig_res.end(); ) {
+    const std::string& seg = start->segment;
+    auto ch = std::find_if(chains.begin(), chains.end(), [&](Chain& c) {
+                return !c.residues.empty() && c.residues[0].segment == seg; });
+    if (ch == chains.end()) {
+      chains.push_back(orig);
+      ch = chains.end() - 1;
+      // Naming. Here, Dup means chain name + segment.
+      switch (namegen.how) {
+        case HowToNameCopiedChain::Short:
+          ch->name = namegen.make_short_name(ch->name + seg);
+          break;
+        case HowToNameCopiedChain::AddNumber:
+          ch->name = namegen.make_name_with_numeric_postfix(ch->name, ++n);
+          break;
+        case HowToNameCopiedChain::Dup:
+          ch->name += seg;
+          break;
+      }
+    }
+    auto end = std::find_if(start, orig_res.end(),
+                            [&](Residue& r) { return r.segment != seg; });
+    ch->residues.insert(ch->residues.end(), std::make_move_iterator(start),
+                                            std::make_move_iterator(end));
+    start = end;
+  }
+  return chains;
+}
+
+void split_chains_by_segments(Model& model, HowToNameCopiedChain how) {
+  ChainNameGenerator namegen(how);
+  std::vector<Chain> new_chains;
+  for (Chain& chain : model.chains)
+    vector_move_extend(new_chains, split_chain_by_segments(chain, namegen));
+  model.chains = std::move(new_chains);
+}
+
+} // namespace gemmi

image_analysis/structure_refinement/ModelValidation.cpp

@@ -22,6 +22,7 @@
 #include <unordered_map>
 
 #include <gemmi/mmread_gz.hpp> // read_structure_gz
+#include <gemmi/assembly.hpp>   // expand_ncs
 #include <gemmi/gz.hpp>        // MaybeGzipped
 #include <gemmi/it92.hpp>      // IT92 x-ray form factors
 #include <gemmi/dencalc.hpp>   // DensityCalculator
@@ -213,6 +214,15 @@ void reorient_about_centroid(gemmi::Model &model, const gemmi::Mat33 &rot) {
     SetModelPositions(model, pos);
 }
 
+// A deposition can hold only part of the asymmetric unit, with the strict-NCS operators that generate
+// the rest (mmCIF _struct_ncs_oper code 'generate', PDB MTRIX without the given flag): an icosahedral
+// virus deposits a few chains of the dozens in the crystal's ASU. Read as it is, the model is that
+// fraction of the crystal, and every R and map made from it describes the fraction.
+void expand_strict_ncs(gemmi::Structure &st) {
+    if (std::any_of(st.ncs.begin(), st.ncs.end(), [](const gemmi::NcsOp &op) { return !op.given; }))
+        gemmi::expand_ncs(st, gemmi::HowToNameCopiedChain::Short);
+}
+
 // Move a model into another cell by keeping its fractional coordinates. Correct only when the two
 // cells describe the same axes in the same order, which is what the probe below is there to arrange.
 void refractionalize_into(gemmi::Structure &st, const gemmi::UnitCell &target) {
@@ -437,6 +447,7 @@ ModelValidationResult Validate(const std::vector<MergedReflection> &merged,
         // falls back to the content when it does not recognise one. A model arrives named however
         // whoever produced it named it, so the file itself is the better authority.
         st = gemmi::read_structure_gz(model_path, gemmi::CoorFormat::Detect);
+        expand_strict_ncs(st);
     } catch (const std::exception &e) {
         result.failure_reason = fmt::format("cannot read model {}: {}", model_path, e.what());
         logger.Error("Model validation: {}", result.failure_reason);
@@ -1819,6 +1830,7 @@ std::vector<MergedReflection> ModelReferenceIntensities(const std::string &model
         // falls back to the content when it does not recognise one. A model arrives named however
         // whoever produced it named it, so the file itself is the better authority.
         st = gemmi::read_structure_gz(model_path, gemmi::CoorFormat::Detect);
+        expand_strict_ncs(st);
     } catch (const std::exception &e) {
         logger.Error("Model reference: cannot read model {}: {}", model_path, e.what());
         return out;

rugnux/RugnuxScaleMerge.cpp

@@ -3727,8 +3727,10 @@ bool Rugnux::ScaleMergeAndSymmetry(PipelineLocals &p) {
                      gate = ahead.write.get_future().share()]() mutable {
                 if (const int32_t cards = get_gpu_count(); cards > 0)
                     pin_gpu(1 % cards);
-                if (!(ahead.indexing_op == gemmi::Op::identity()))
+                if (!(ahead.indexing_op == gemmi::Op::identity())) {
                     merged = ReindexMergedIntoAsu(merged, ahead.indexing_op, sg, friedel);
+                    cell = CellInBasis(cell, HklOperator(ahead.indexing_op));
+                }
                 gemmi::Op to_model = ahead.change_of_basis_op;
                 to_model.tran = {0, 0, 0};
                 const gemmi::Op cob = to_model.inverse();
@@ -3792,6 +3794,7 @@ bool Rugnux::ScaleMergeAndSymmetry(PipelineLocals &p) {
                 result.merge_statistics_text.erase(pos, ambiguity_warning_text.size());
             logger.Info("Indexing ambiguity resolved against the model; warning withdrawn");
         }
+        bool indexing_moved_cell = false;
         if (data_sg.has_value()) {
             const gemmi::SpaceGroup *adopted =
                 AdoptModelFrame(validation, sm.merged, *data_sg,
@@ -3808,13 +3811,14 @@ bool Rugnux::ScaleMergeAndSymmetry(PipelineLocals &p) {
             // The unmerged export below comes from the integrated observations, not from the merge,
             // so it follows the same relabeling - otherwise the two files would describe different
             // indexings of one dataset. The enantiomorph is a label, so only the indexing moves it.
+            // The cell moves with the indices: an alternative indexing that is only a near-symmetry of
+            // the lattice (b within 2% of c, beta within a degree of 90) permutes the axes or flips an
+            // angle, and the written cell has to be the one the written indices are on.
             const gemmi::Op &op = validation.indexing_op;
             if (!(op == gemmi::Op::identity())) {
-                for (auto &io : indexer->GetIntegrationOutcome())
-                    for (auto &r : io.reflections) {
-                        const gemmi::Op::Miller h = op.apply_to_hkl({{r.h, r.k, r.l}});
-                        r.h = h[0]; r.k = h[1]; r.l = h[2];
-                    }
+                const UnitCell before = *result.consensus_cell;
+                reseat_integrated(HklOperator(op));
+                indexing_moved_cell = !(*result.consensus_cell == before);
                 merge_to_written = merge_to_written.combine(op);
                 determined_to_written = determined_to_written.combine(op);
                 result.setting_operator = IndexTriplet(determined_to_written);
@@ -3826,8 +3830,10 @@ bool Rugnux::ScaleMergeAndSymmetry(PipelineLocals &p) {
         // validation is then made again on the relabelled data, so the maps and the placed model
         // come out on the same axes as the reflections; what it had decided is kept, since the
         // second one, facing a model already in the data's setting, has nothing left to decide.
+        // The same where the model's indexing moved the cell (a near-symmetry, above): the first
+        // validation placed the model in the cell as it was before.
         const bool remake = config_.reference_data.empty() && validation.model_fits
-                            && !(validation.change_of_basis_op == gemmi::Op::identity());
+                            && (!(validation.change_of_basis_op == gemmi::Op::identity()) || indexing_moved_cell);
         const bool forecast_held = forecast_kept(validation);
         if (ahead.run.valid() && !forecast_held) {
             ahead.write.set_value(false);
@@ -3843,7 +3849,8 @@ bool Rugnux::ScaleMergeAndSymmetry(PipelineLocals &p) {
                 relabel_output(to_model.inverse(), "MODEL", &ahead.merged);
                 ahead.log.ReplayInto(logger);
             } else {
-                relabel_output(to_model.inverse(), "MODEL");
+                if (!(to_model == gemmi::Op::identity()))
+                    relabel_output(to_model.inverse(), "MODEL");
                 const auto sg_now = experiment_.GetGemmiSpaceGroup();
                 remade = ValidateAgainstModel(sm.merged, *result.consensus_cell, config_.model_path,
                                               config_.output_prefix, logger,
@@ -3899,8 +3906,10 @@ bool Rugnux::ScaleMergeAndSymmetry(PipelineLocals &p) {
                                                           twin_file->space_group,
                                                           experiment_.GetScalingSettings().GetMergeFriedel(),
                                                           logger);
-                if (!(op == gemmi::Op::identity()))
+                if (!(op == gemmi::Op::identity())) {
                     twin_file->law = op.inverse().combine(twin_file->law).combine(op);
+                    twin_file->cell = CellInBasis(twin_file->cell, HklOperator(op));
+                }
                 result.twin_subgroup_merge->twin_law = twin_file->law.as_hkl().triplet();
                 result.twin_subgroup_merge->space_group = twin_file->space_group.xhm();
             }

rugnux/rugnux_cli.cpp

@@ -52,6 +52,7 @@
 #include "../image_analysis/SensorAbsorption.h"
 #include "../image_analysis/UpdateReflectionResolution.h"
 #include "../image_analysis/WriteReflections.h"
+#include "../image_analysis/scale_merge/CrystalSetting.h"   // CellInBasis, HklOperator
 #include "../image_analysis/scale_merge/Merge.h"
 #include "../image_analysis/scale_merge/RfreeFlags.h"
 #include "../image_analysis/scale_merge/ReindexAmbiguity.h"   // MatchReference
@@ -2221,13 +2222,16 @@ static int RunRugnux(int argc, char **argv) {
                 // The unmerged export below writes the integrated observations, so it follows the same
                 // relabeling as the merged set - only the indexing moves a reflection; the enantiomorph
                 // is a label - or the two files would describe different indexings of one dataset.
+                // The cell moves with the indices too, for an indexing that is only a near-symmetry.
                 const gemmi::Op &op = validation.indexing_op;
-                if (!(op == gemmi::Op::identity()))
+                if (!(op == gemmi::Op::identity())) {
                     for (auto &io : reflections)
                         for (auto &r : io.reflections) {
                             const gemmi::Op::Miller h = op.apply_to_hkl({{r.h, r.k, r.l}});
                             r.h = h[0]; r.k = h[1]; r.l = h[2];
                         }
+                    experiment.SetUnitCell(CellInBasis(*experiment.GetUnitCell(), HklOperator(op)));
+                }
             }
             // The coordinates that go with the maps just written, in the frame the reflection files
             // beside them carry. Written whenever the validation ran, not only where the rigid-body

tests/ModelValidationTest.cpp

@@ -134,6 +134,46 @@ TEST_CASE("ModelValidation_ReadsPdbAndMmcif", "[ModelValidation]") {
     std::filesystem::remove(misnamed);
 }
 
+// A deposition can hold part of the asymmetric unit and the strict-NCS operators that generate the rest
+// (MTRIX without the given flag). The model it describes is the expanded one: here the two atoms and
+// their copy half a cell along a, which the explicit four-atom model spells out.
+TEST_CASE("ModelValidation_ExpandsStrictNcs", "[ModelValidation]") {
+    Logger logger("ModelValidation_ExpandsStrictNcs");
+
+    const char *with_ncs =
+        "CRYST1   40.000   50.000   60.000  90.00  90.00  90.00 P 1           1\n"
+        "MTRIX1   1  1.000000  0.000000  0.000000        0.00000    1\n"
+        "MTRIX2   1  0.000000  1.000000  0.000000        0.00000    1\n"
+        "MTRIX3   1  0.000000  0.000000  1.000000        0.00000    1\n"
+        "MTRIX1   2  1.000000  0.000000  0.000000       20.00000\n"
+        "MTRIX2   2  0.000000  1.000000  0.000000        0.00000\n"
+        "MTRIX3   2  0.000000  0.000000  1.000000        0.00000\n"
+        "ATOM      1  CA  GLY A   1      10.000  12.000  14.000  1.00 20.00           C\n"
+        "ATOM      2  CB  GLY A   1      12.000  14.000  16.000  1.00 20.00           C\n"
+        "END\n";
+    const char *expanded =
+        "CRYST1   40.000   50.000   60.000  90.00  90.00  90.00 P 1           1\n"
+        "ATOM      1  CA  GLY A   1      10.000  12.000  14.000  1.00 20.00           C\n"
+        "ATOM      2  CB  GLY A   1      12.000  14.000  16.000  1.00 20.00           C\n"
+        "ATOM      3  CA  GLY B   1      30.000  12.000  14.000  1.00 20.00           C\n"
+        "ATOM      4  CB  GLY B   1      32.000  14.000  16.000  1.00 20.00           C\n"
+        "END\n";
+    const auto ncs_path = WriteTemp("model_validation_test_ncs.pdb", with_ncs);
+    const auto full_path = WriteTemp("model_validation_test_ncs_full.pdb", expanded);
+
+    const auto from_ncs = ModelReferenceIntensities(ncs_path, {}, {}, 4.0, logger);
+    const auto from_full = ModelReferenceIntensities(full_path, {}, {}, 4.0, logger);
+    REQUIRE(from_ncs.size() == from_full.size());
+    REQUIRE_FALSE(from_ncs.empty());
+    for (size_t i = 0; i < from_ncs.size(); i++) {
+        CHECK(from_ncs[i].h == from_full[i].h);
+        CHECK(from_ncs[i].I == Catch::Approx(from_full[i].I).margin(1e-3 * from_full[i].I + 1e-6));
+    }
+
+    std::filesystem::remove(ncs_path);
+    std::filesystem::remove(full_path);
+}
+
 // A model that cannot be used must say why. Returning an empty result and logging was enough to lose
 // the fact entirely: the run finished successfully with no R-free and no maps, which is exactly what
 // a run without --model looks like.

tests/ReindexAmbiguityTest.cpp

@@ -3,10 +3,12 @@
 
 #include <catch2/catch_all.hpp>
 
+#include <cmath>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
 
+#include "../image_analysis/scale_merge/CrystalSetting.h"
 #include "../image_analysis/scale_merge/HKLKey.h"
 #include "../image_analysis/scale_merge/ReindexAmbiguity.h"
 
@@ -212,3 +214,28 @@ TEST_CASE("Reindex: every ambiguity operator is a proper rotation", "[reindex]")
             CHECK(op.det_rot() == gemmi::Op::DEN * gemmi::Op::DEN * gemmi::Op::DEN);
     }
 }
+
+TEST_CASE("Reindex: a near-symmetry law moves the cell with the indices", "[reindex]") {
+    // Pseudo-merohedry: b within 2% of c in a primitive orthorhombic cell, and beta a degree off 90 in
+    // a monoclinic one. Each admits an indexing law that is not a symmetry of the cell, so relabelling
+    // the reflections by it permutes the axes (or flips beta), and the cell they are written with has
+    // to follow - every reflection then keeps its own d-spacing.
+    const std::vector<std::pair<UnitCell, int>> cases{
+        {UnitCell{50.0f, 60.0f, 61.2f, 90.0f, 90.0f, 90.0f}, 16},    // P222, b ~ c
+        {UnitCell{70.0f, 79.0f, 83.0f, 90.0f, 91.1f, 90.0f}, 3},     // P2, beta ~ 90
+    };
+    for (const auto &[cell, sg] : cases) {
+        const auto laws = ReindexAmbiguityOperators(cell, SG(sg));
+        REQUIRE_FALSE(laws.empty());
+        bool moved = false;
+        for (const gemmi::Op &law : laws) {
+            const UnitCell written = CellInBasis(cell, HklOperator(law));
+            moved = moved || std::abs(written.b - cell.b) > 0.5f || std::abs(written.beta - cell.beta) > 0.5f;
+            const gemmi::UnitCell before = cell, after = written;
+            for (const gemmi::Op::Miller hkl : {gemmi::Op::Miller{1, 2, 3}, gemmi::Op::Miller{-4, 0, 7},
+                                                gemmi::Op::Miller{5, -3, 1}})
+                CHECK(after.calculate_d(law.apply_to_hkl(hkl)) == Catch::Approx(before.calculate_d(hkl)).epsilon(1e-5));
+        }
+        CHECK(moved);
+    }
+}

tools/battery/depdata_check.py

@@ -134,7 +134,7 @@ def check(mtz_path, pdb_id, cache_dir=model_check.DEFAULT_CACHE, wavelength=None
                                os.path.join(cache_dir, pdb_id + "-sf.cif.gz"))
         if sf is None:
             return dict(res, dep_status="n/a", dep_reason="no deposited structure factors")
-        st = gemmi.read_structure(meta["xyz"])
+        st = model_check.read_model(meta["xyz"])
         sg_m, cell_m = st.find_spacegroup(), st.cell
         if sg_m is None:
             return dict(res, dep_reason=f"model space group '{st.spacegroup_hm}' not recognised")

tools/battery/model_check.py

@@ -404,6 +404,16 @@ def refined_shared(workdir, data, dep, cell_m, sg_m, xyz, twin):
     return {k_: (round(v, 4) if isinstance(v, float) else v) for k_, v in out.items()}
 
 
+def read_model(xyz):
+    """The deposited model with its strict-NCS copies generated: an entry can hold part of the
+    asymmetric unit and the operators that make the rest (_struct_ncs_oper 'generate', MTRIX without
+    the given flag) - an icosahedral virus deposits a few chains of dozens."""
+    st = gemmi.read_structure(xyz)
+    if any(not op.given for op in st.ncs):
+        st.expand_ncs(gemmi.HowToNameCopiedChain.Short)
+    return st
+
+
 def model_for_refmac(st, workdir):
     """The deposited model written in a form REFMAC reads, unchanged in everything it scores;
     returns the file and what had to be changed ("none" normally)."""
@@ -587,7 +597,7 @@ def check(mtz_path, pdb_id, workdir, cache_dir=DEFAULT_CACHE, baseline=True, ref
             return done("n/a", "not an X-ray structure: " + ", ".join(meta["methods"]))
         if meta["rfree"] is None:
             return done("n/a", "no deposited R-free")
-        st = gemmi.read_structure(meta["xyz"])
+        st = read_model(meta["xyz"])
         if len(st) > 1:
             return done("skipped", f"{len(st)} models in the entry")
         natoms = st[0].count_atom_sites()