From 122cd6e459bf608a50bdb8f4800b222bf8ecaecf Mon Sep 17 00:00:00 2001 From: Clemens Schmid Date: Tue, 24 Mar 2026 14:09:25 +0100 Subject: [PATCH 1/3] Fix CIF rhombohedral symmetry scoring mutating imported lattice Score alternate CIF space-group settings on a temporary Crystal instead of the live imported object, preventing :R/:H probing from corrupting rhombohedral hexagonal cell metrics during pyobjcryst CIF import. happened with "_symmetry_equiv_pos_as_xyz" keyword in CIFs --- ObjCryst/ObjCryst/CIF.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/ObjCryst/ObjCryst/CIF.cpp b/ObjCryst/ObjCryst/CIF.cpp index 8fd79cb..7085e29 100644 --- a/ObjCryst/ObjCryst/CIF.cpp +++ b/ObjCryst/ObjCryst/CIF.cpp @@ -1130,10 +1130,15 @@ Crystal* CreateCrystalFromCIF(CIF &cif,const bool verbose,const bool checkSymAsX unsigned int bestscore=0; for(vector::const_iterator posOrig=origin_list.begin();posOrig!=origin_list.end();++posOrig) { - // The origin extension may not make sense, so we need to watch for exception + // Test candidate settings on a temporary crystal so we don't mutate the + // imported crystal lattice while probing alternate origin/setting choices. + Crystal tmpCryst(pos->second.mvLatticePar[0],pos->second.mvLatticePar[1],pos->second.mvLatticePar[2], + pos->second.mvLatticePar[3],pos->second.mvLatticePar[4],pos->second.mvLatticePar[5],hmorig); + string testSymbol=hmorig; try { - pCryst->ChangeSpaceGroup(hmorig+*posOrig); + testSymbol += *posOrig; + tmpCryst.ChangeSpaceGroup(testSymbol); } catch(invalid_argument) { @@ -1141,9 +1146,9 @@ Crystal* CreateCrystalFromCIF(CIF &cif,const bool verbose,const bool checkSymAsX } // If the symbol is the same as before, the origin probably was not understood - no need to test - if((posOrig!=origin_list.begin())&&(pCryst->GetSpaceGroup().GetName()==bestsymbol)) continue; + if((posOrig!=origin_list.begin())&&(tmpCryst.GetSpaceGroup().GetName()==bestsymbol)) continue; - unsigned int nbSymSpg=pCryst->GetSpaceGroup().GetCCTbxSpg().all_ops().size(); + unsigned int nbSymSpg=tmpCryst.GetSpaceGroup().GetCCTbxSpg().all_ops().size(); unsigned int nbSymCommon=0; try { @@ -1153,7 +1158,7 @@ Crystal* CreateCrystalFromCIF(CIF &cif,const bool verbose,const bool checkSymAsX posSymCIF!=pos->second.mvSymmetry_equiv_pos_as_xyz.end();++posSymCIF) { cctbx::sgtbx::rt_mx mx1(*posSymCIF); - cctbx::sgtbx::rt_mx mx2(pCryst->GetSpaceGroup().GetCCTbxSpg().all_ops()[i]); + cctbx::sgtbx::rt_mx mx2(tmpCryst.GetSpaceGroup().GetCCTbxSpg().all_ops()[i]); mx1.mod_positive_in_place(); mx2.mod_positive_in_place(); if(mx1==mx2) @@ -1163,14 +1168,14 @@ Crystal* CreateCrystalFromCIF(CIF &cif,const bool verbose,const bool checkSymAsX } } } - if(verbose) cout<<" Trying: "<GetSpaceGroup().GetName() + if(verbose) cout<<" Trying: "<second.mvSymmetry_equiv_pos_as_xyz.size()<<"(CIF)" <<",common:"<second.mvSymmetry_equiv_pos_as_xyz.size())*nbSymCommon)) { bestscore=(nbSymSpg==pos->second.mvSymmetry_equiv_pos_as_xyz.size())*nbSymCommon; - bestsymbol=pCryst->GetSpaceGroup().GetName(); + bestsymbol=tmpCryst.GetSpaceGroup().GetName(); } } catch(cctbx::error) From 7c8175c197f72b317477d5eb9d6c3d6bbefdccb2 Mon Sep 17 00:00:00 2001 From: Clemens Schmid Date: Mon, 30 Mar 2026 15:00:52 +0200 Subject: [PATCH 2/3] use space_group instead of temp crystal --- ObjCryst/ObjCryst/CIF.cpp | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/ObjCryst/ObjCryst/CIF.cpp b/ObjCryst/ObjCryst/CIF.cpp index 7085e29..9dc8186 100644 --- a/ObjCryst/ObjCryst/CIF.cpp +++ b/ObjCryst/ObjCryst/CIF.cpp @@ -1130,25 +1130,25 @@ Crystal* CreateCrystalFromCIF(CIF &cif,const bool verbose,const bool checkSymAsX unsigned int bestscore=0; for(vector::const_iterator posOrig=origin_list.begin();posOrig!=origin_list.end();++posOrig) { - // Test candidate settings on a temporary crystal so we don't mutate the - // imported crystal lattice while probing alternate origin/setting choices. - Crystal tmpCryst(pos->second.mvLatticePar[0],pos->second.mvLatticePar[1],pos->second.mvLatticePar[2], - pos->second.mvLatticePar[3],pos->second.mvLatticePar[4],pos->second.mvLatticePar[5],hmorig); - string testSymbol=hmorig; + string testSymbol=hmorig + *posOrig; + string canonicalSymbol; + cctbx::sgtbx::space_group cctbxspg; try { - testSymbol += *posOrig; - tmpCryst.ChangeSpaceGroup(testSymbol); + cctbxspg = cctbx::sgtbx::space_group(cctbx::sgtbx::space_group_symbols(testSymbol)); + const char ext = cctbxspg.match_tabulated_settings().extension(); + canonicalSymbol = cctbxspg.match_tabulated_settings().hermann_mauguin(); + if(ext!='\0') canonicalSymbol += ":" + string(1, ext); } - catch(invalid_argument) + catch(exception) { continue; } // If the symbol is the same as before, the origin probably was not understood - no need to test - if((posOrig!=origin_list.begin())&&(tmpCryst.GetSpaceGroup().GetName()==bestsymbol)) continue; + if((posOrig!=origin_list.begin())&&(canonicalSymbol==bestsymbol)) continue; - unsigned int nbSymSpg=tmpCryst.GetSpaceGroup().GetCCTbxSpg().all_ops().size(); + unsigned int nbSymSpg=cctbxspg.all_ops().size(); unsigned int nbSymCommon=0; try { @@ -1158,7 +1158,7 @@ Crystal* CreateCrystalFromCIF(CIF &cif,const bool verbose,const bool checkSymAsX posSymCIF!=pos->second.mvSymmetry_equiv_pos_as_xyz.end();++posSymCIF) { cctbx::sgtbx::rt_mx mx1(*posSymCIF); - cctbx::sgtbx::rt_mx mx2(tmpCryst.GetSpaceGroup().GetCCTbxSpg().all_ops()[i]); + cctbx::sgtbx::rt_mx mx2(cctbxspg.all_ops()[i]); mx1.mod_positive_in_place(); mx2.mod_positive_in_place(); if(mx1==mx2) @@ -1168,14 +1168,14 @@ Crystal* CreateCrystalFromCIF(CIF &cif,const bool verbose,const bool checkSymAsX } } } - if(verbose) cout<<" Trying: "<second.mvSymmetry_equiv_pos_as_xyz.size()<<"(CIF)" <<",common:"<second.mvSymmetry_equiv_pos_as_xyz.size())*nbSymCommon)) { bestscore=(nbSymSpg==pos->second.mvSymmetry_equiv_pos_as_xyz.size())*nbSymCommon; - bestsymbol=tmpCryst.GetSpaceGroup().GetName(); + bestsymbol=canonicalSymbol; } } catch(cctbx::error) From 7c6bf98a83a12ca1f5180ff82f12a634f9824fff Mon Sep 17 00:00:00 2001 From: Clemens Schmid Date: Mon, 30 Mar 2026 16:20:46 +0200 Subject: [PATCH 3/3] More specific catch for invalid sgtbx symbols during symmetry scoring --- ObjCryst/ObjCryst/CIF.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ObjCryst/ObjCryst/CIF.cpp b/ObjCryst/ObjCryst/CIF.cpp index 9dc8186..4321f23 100644 --- a/ObjCryst/ObjCryst/CIF.cpp +++ b/ObjCryst/ObjCryst/CIF.cpp @@ -1140,7 +1140,7 @@ Crystal* CreateCrystalFromCIF(CIF &cif,const bool verbose,const bool checkSymAsX canonicalSymbol = cctbxspg.match_tabulated_settings().hermann_mauguin(); if(ext!='\0') canonicalSymbol += ":" + string(1, ext); } - catch(exception) + catch(cctbx::error) { continue; }