Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rigid Group with multiple atoms leads to an invalid XML #52

Closed
kuba-v opened this issue Feb 12, 2022 · 1 comment
Closed

Rigid Group with multiple atoms leads to an invalid XML #52

kuba-v opened this issue Feb 12, 2022 · 1 comment
Assignees

Comments

@kuba-v
Copy link

kuba-v commented Feb 12, 2022

When a Rigid Group is added to a molecule and more then one atom is selected the XML representing the setting has multiple attributes of the name "Atom". For example:
<RigidGroup Atom="N1" Atom="N2" Atom="C2"/>
This is an invalid XML as in XML definition says that attribute names should be unique (see e.g. here Wikipedia XML Attribute).
If such configuration is added it is not possible to open or edit the XML with most XML editors or other frameworks processing XMLs.

@vincefn
Copy link
Owner

vincefn commented Feb 14, 2022

Hi - thanks for pointing this out. The use of XML for objcryst/Fox cut a few corners in the design - specifically the xml data cannot be properly validated because I needed a few constraints to include in a true validation (specific elements inside other elements, array of diffraction data of size NxM without the overhead of using an XML element for each value and each row,...), and it was not possible to create a DTD with all those constraints (at least when I wrote this part of the code).

Nevertheless, the XML should be well-formed and this is indeed not the case when there is a RigidGroup. It's easy enough to correct in the code, but the new files will not be backwards-compatible. Not sure if this would affect many people.

PS: unrelated comment - I'm not too fond of rigid groups - they are not very efficient ate helping to find a structure solution and they will not stay strictly rigid (notably during LSQ).

@vincefn vincefn self-assigned this Feb 14, 2022
vincefn added a commit to vincefn/libobjcryst that referenced this issue Aug 28, 2022
* PowderPatternDiffraction: add GetFhklObsSq() and HasFhklObsSq()
* Use a valid XML output for a Molecule RigidGroup, writing atoms as Atom1, Atom2 etc... instead of repeating the Atom attribute. Fixes vincefn/objcryst#52. The saved files will not be backwards-compatible (readable but the list of atoms in the rigid group will not be read)
* Fix double loop in PowderPattern::PrepareIntegratedRfactor.
vincefn added a commit to diffpy/libobjcryst that referenced this issue Nov 2, 2022
…jcryst

* 'master' of github.com:vincefn/objcryst:
  Enable automatic re-generation of HKL's for a PowderPatternDiffraction when the spacegroup is changed, or when the lattice parameters change (the latter only outside of an optimisation). This is done by making all the HKL attributes mutable, and overloading the SetHKL and GenHKLFullSpace, GenHKLFullSpace2 with a protected const version, which should only be used for powder diffraction (and not single crystal data). This should address diffpy/pyobjcryst#32
  Update makefile to wxwidgets 3.2 and fftw to 3.3.10 ; update macOS Xcode project to make a universal binary with X86_64 and ARM64 architectures
  RefinableObj: change list<> to std::list<> to avoid compiler ambiguity with boost list.
  PowderPatternDiffraction: add GetFhklObsSq() and HasFhklObsSq()
  Correct update check code for YYYYNNN format.
  disable glcanvasegl for wxWindgets compilation under linux to avoid linking errors (?)
  Update wiki2pdf.py for python 3
  Update Fox version. Add fenske-hall z-matrix with hydrogens to cimetidine tutorial. Update macOS build config and makefile with wxwidgets 3.1.6. Remove obsolete makefiles.
  Update changelog and windows project files
  Use a valid XML output for a Molecule RigidGroup, writing atoms as Atom1, Atom2 etc... instead of repeating the Atom attribute. Fixes vincefn/objcryst#52. The saved files will not be backwards-compatible (readable but the list of atoms in the rigid group will not be read)
  Fix double loop in PowderPattern::PrepareIntegratedRfactor.
  Better GetFormula for Crystal and Molecule
  Crystal::XMLInput(): add a hook to re-use atomic scattering power when mDeleteSubObjInDestructor is False. This is useful when re-loading Crystal configurations using their xml descriptions repeatedly, notably in python.
  Add relative_length_tolerance and absolute_angle_tolerance_degree to SpaceGroupExplorer::Run() and RunAll()
  Correct precision for Crystal::GetFormula()
  Add access to the weight (g/mol) for ScatteringPowerAtom and Crystal
  Add access to the weight (g/mol) for ScatteringPowerAtom and Crystal
  CreateCrystalFromCIF: throw exception if no crystal structure is found
  Add int_ptr() function to enable unique identification of objects through their address - useful in python where the wrapped object has a different address
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

No branches or pull requests

2 participants