Merge branch 'trunk' into dict-for-data

CMakeLists.txt

@@ -27,7 +27,7 @@ cmake_minimum_required(VERSION 3.23)
 # set the project name
 project(
 	libcifpp
-	VERSION 7.0.7
+	VERSION 8.0.0
 	LANGUAGES CXX)
 
 list(PREPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
@@ -41,6 +41,12 @@ include(CheckCXXSourceCompiles)
 include(GenerateExportHeader)
 include(CTest)
 include(FetchContent)
+include(ExternalProject)
+
+# FindBoost, take care of it now.
+if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.30)
+	cmake_policy(SET CMP0167 NEW)
+endif()
 
 # When building with ninja-multiconfig, build both debug and release by default
 if(CMAKE_GENERATOR STREQUAL "Ninja Multi-Config")
@@ -231,21 +237,18 @@ if(Eigen3_FOUND AND TARGET Eigen3::Eigen)
 	get_target_property(EIGEN_INCLUDE_DIR Eigen3::Eigen
 		INTERFACE_INCLUDE_DIRECTORIES)
 else()
-	# Create a private copy of eigen3 and populate it only, no need to build
-	FetchContent_Declare(
-		my-eigen3
+	# Use ExternalProject since FetchContent always tries to install the result...
+	ExternalProject_Add(my-eigen3
 		GIT_REPOSITORY https://gitlab.com/libeigen/eigen.git
-		GIT_TAG 3.4.0)
-
-	FetchContent_GetProperties(my-eigen3)
-
-	if(NOT my-eigen3_POPULATED)
-		FetchContent_Populate(my-eigen3)
-	endif()
-
-	set(EIGEN_INCLUDE_DIR ${my-eigen3_SOURCE_DIR})
+		GIT_TAG 3.4.0
+		INSTALL_COMMAND "")
+	
+	ExternalProject_Get_Property(my-eigen3 SOURCE_DIR)
+	set(EIGEN_INCLUDE_DIR ${SOURCE_DIR})
 endif()
 
+message(STATUS "Eigen include dir is ${EIGEN_INCLUDE_DIR}")
+
 # Create a revision file, containing the current git version info
 include(VersionString)
 write_version_header(${CMAKE_CURRENT_SOURCE_DIR}/src/ LIB_NAME "LibCIFPP")
@@ -332,6 +335,10 @@ set(project_headers
 add_library(cifpp)
 add_library(cifpp::cifpp ALIAS cifpp)
 
+if(TARGET my-eigen3)
+	add_dependencies(cifpp my-eigen3)
+endif()
+
 target_sources(cifpp
 	PRIVATE ${project_sources}
 	${CMAKE_CURRENT_SOURCE_DIR}/src/symop_table_data.hpp
@@ -534,7 +541,7 @@ write_basic_package_version_file(
 	VERSION ${PROJECT_VERSION}
 	COMPATIBILITY AnyNewerVersion)
 
-if(BUILD_TESTING)
+if(BUILD_TESTING AND PROJECT_IS_TOP_LEVEL)
 	add_subdirectory(test)
 endif()
 

README.md

@@ -32,7 +32,7 @@ The documentation can be found at [github.io](https://pdb-redo.github.io/libcifp
 
 ## Synopsis
 
-```c++
+```cpp
 // A simple program counting residues with an OXT atom
 
 #include <filesystem>

changelog

@@ -1,3 +1,26 @@
+Version 8.0.0
+- A dictionary is for a datablock and a file can have
+  datablocks with differing dictionaries.
+
+Version 7.0.10
+- Deal with missing _entity.type in reconstructing mmCIF files
+- Replace code creating quaternions from rotation matrices
+  that might sometimes give incorrect results. Or at least,
+  the test code failed on this particular kind of code. Sometimes.
+- Fix reconstruction to build pdbx_nonpoly_scheme
+
+Version 7.0.9
+- Using cif::file::load_dictionary it is now possible to
+  load a dictionary along with its extensions in one go.
+  E.g. file.load_dictionary("mmcif_pdbx;dssp-extension")
+- Fix in compound factory to avoid errors with lower case
+  compound id's
+- Fix sac_parser's index to be case insensitive
+
+Version 7.0.8
+- Fix PDB Remark 3 parser
+- Added three way comparison for point
+
 Version 7.0.7
 - Set CIFPP_DATA_DIR on target cifpp for use in projects that include
   libcifpp directly

docs/index.rst

@@ -27,7 +27,7 @@ Using *libcifpp* is easy, if you are familiar with modern C++:
 
 .. literalinclude:: ../README.md
 	:language: c++
-	:start-after: ```c++
+	:start-after: ```cpp
 	:end-before: ```
 
 .. toctree::

include/cif++/compound.hpp

@@ -180,8 +180,7 @@ class compound
 	friend class local_compound_factory_impl;
 
 	compound(cif::datablock &db);
-	compound(cif::datablock &db, int);
-
+	
 	std::string m_id;
 	std::string m_name;
 	std::string m_type;

include/cif++/datablock.hpp

@@ -98,6 +98,19 @@ class datablock : public std::list<category>
 		m_name = name;
 	}
 
+	/**
+	 * @brief Attempt to load the dictionary specified in audit_conform category
+	 * 
+	 */
+	void load_dictionary();
+
+	/**
+	 * @brief Load the dictionary named @a dict_name
+	 * 
+	 * @param dict_name 
+	 */
+	void load_dictionary(std::string_view dict_name);
+
 	/**
 	 * @brief Set the validator object to @a v
 	 * 

include/cif++/file.hpp

@@ -118,19 +118,6 @@ class file : public std::list<datablock>
 
 	/** @endcond */
 
-	/**
-	 * @brief Set the validator object to @a v
-	 */
-	void set_validator(const validator *v);
-
-	/**
-	 * @brief Get the validator object
-	 */
-	const validator *get_validator() const
-	{
-		return m_validator;
-	}
-
 	/**
 	 * @brief Validate the content and return true if everything was valid.
 	 * 
@@ -165,21 +152,6 @@ class file : public std::list<datablock>
 	 */
 	bool validate_links() const;
 
-	/**
-	 * @brief Attempt to load a dictionary (validator) based on
-	 * the contents of the *audit_conform* category, if available.
-	 */
-	void load_dictionary();
-
-
-	/**
-	 * @brief Attempt to load the named dictionary @a name and
-	 * create a validator based on it.
-	 * 
-	 * @param name The name of the dictionary to load
-	 */
-	void load_dictionary(std::string_view name);
-
 	/**
 	 * @brief Return true if a datablock with the name @a name is part of this file
 	 */
@@ -232,6 +204,18 @@ class file : public std::list<datablock>
 	/** Load the data from @a is */
 	void load(std::istream &is);
 
+	/** Load the data from the file specified by @a p using validator @a v */
+	void load(const std::filesystem::path &p, const validator &v);
+
+	/** Load the data from @a is using validator @a v */
+	void load(std::istream &is, const validator &v);
+
+	/** Load the data from the file specified by @a p using a validator constructed from dictionary @a dict */
+	void load(const std::filesystem::path &p, std::string_view dict);
+
+	/** Load the data from @a is using a validator constructed from dictionary @a dict */
+	void load(std::istream &is, std::string_view dict);
+
 	/** Save the data to the file specified by @a p */
 	void save(const std::filesystem::path &p) const;
 
@@ -246,9 +230,6 @@ class file : public std::list<datablock>
 		f.save(os);
 		return os;
 	}
-
-  private:
-	const validator *m_validator = nullptr;
 };
 
 } // namespace cif

include/cif++/parser.hpp

@@ -39,6 +39,8 @@
 namespace cif
 {
 
+class validator;
+
 // --------------------------------------------------------------------
 
 /** Exception that is thrown when the mmCIF file contains a parsing error */
@@ -73,7 +75,15 @@ class sac_parser
 {
   public:
 	/** @cond */
-	using datablock_index = std::map<std::string, std::size_t>;
+	struct iless_op
+	{
+		bool operator()(std::string_view a, std::string_view b) const
+		{
+			return icompare(a, b) < 0;
+		}
+	};
+
+	using datablock_index = std::map<std::string, std::size_t, iless_op>;
 
 	virtual ~sac_parser() = default;
 	/** @endcond */
@@ -299,6 +309,14 @@ class sac_parser
 class parser : public sac_parser
 {
   public:
+	/// \brief constructor, generates data into @a file from @a is using validator @a v
+	parser(std::istream &is, file &file, const validator *v)
+		: sac_parser(is)
+		, m_file(file)
+		, m_validator(v)
+	{
+	}
+
 	/// \brief constructor, generates data into @a file from @a is
 	parser(std::istream &is, file &file)
 		: sac_parser(is)
@@ -319,6 +337,7 @@ class parser : public sac_parser
 	file &m_file;
 	datablock *m_datablock = nullptr;
 	category *m_category = nullptr;
+	const validator *m_validator = nullptr;
 	row_handle m_row;
 
 	/** @endcond */

include/cif++/point.hpp

@@ -662,12 +662,23 @@ struct point_type
 		return std::make_tuple(std::ref(m_x), std::ref(m_y), std::ref(m_z));
 	}
 
-	/// \brief Compare with @a rhs
+#if defined(__cpp_impl_three_way_comparison)
+	/// \brief a default spaceship operator
+	constexpr auto operator<=>(const point_type &rhs) const = default;
+#else
+	/// \brief a default equals operator
 	constexpr bool operator==(const point_type &rhs) const
 	{
 		return m_x == rhs.m_x and m_y == rhs.m_y and m_z == rhs.m_z;
 	}
 
+	/// \brief a default not-equals operator
+	constexpr bool operator!=(const point_type &rhs) const
+	{
+		return not operator==(rhs);
+	}
+#endif
+
 	// consider point as a vector... perhaps I should rename point?
 
 	/// \brief looking at the point as if it is a vector, return the squared length

src/category.cpp

@@ -718,7 +718,7 @@ bool category::is_valid() const
 		auto iv = m_cat_validator->get_validator_for_item(col.m_name);
 		if (iv == nullptr)
 		{
-			m_validator->report_error(validation_error::unknown_item, col.m_name, m_name, false);
+			m_validator->report_error(validation_error::unknown_item, m_name, col.m_name, false);
 			result = false;
 		}
 

src/compound.cpp

@@ -177,89 +177,6 @@ compound::compound(cif::datablock &db)
 	}
 }
 
-compound::compound(cif::datablock &db, int)
-{
-	auto &chemComp = db["chem_comp"];
-
-	if (chemComp.size() != 1)
-		throw std::runtime_error("Invalid compound file, chem_comp should contain a single row");
-
-	cif::tie(m_id, m_name) =
-		chemComp.front().get("id", "name");
-	
-	cif::trim(m_name);
-
-	m_type = "NON-POLYMER";
-
-	auto &chemCompAtom = db["chem_comp_atom"];
-	for (auto row : chemCompAtom)
-	{
-		compound_atom atom;
-		std::string type_symbol;
-		cif::tie(atom.id, type_symbol, atom.charge, atom.x, atom.y, atom.z) =
-			row.get("atom_id", "type_symbol", "charge", "x", "y", "z");
-		atom.type_symbol = atom_type_traits(type_symbol).type();
-
-		m_formal_charge += atom.charge;
-
-		m_atoms.push_back(std::move(atom));
-	}
-
-	auto &chemCompBond = db["chem_comp_bond"];
-	for (auto row : chemCompBond)
-	{
-		compound_bond bond;
-		std::string btype;
-		cif::tie(bond.atom_id[0], bond.atom_id[1], btype, bond.aromatic) = row.get("atom_id_1", "atom_id_2", "type", "aromatic");
-
-		using cif::iequals;
-
-		if (iequals(btype, "single"))
-			bond.type = bond_type::sing;
-		else if (iequals(btype, "double"))
-			bond.type = bond_type::doub;
-		else if (iequals(btype, "triple"))
-			bond.type = bond_type::trip;
-		else if (iequals(btype, "deloc") or iequals(btype, "aromat") or iequals(btype, "aromatic"))
-			bond.type = bond_type::delo;
-		else
-		{
-			if (cif::VERBOSE > 0)
-				std::cerr << "Unimplemented chem_comp_bond.type " << btype << " in " << db.name() << '\n';
-			bond.type = bond_type::sing;
-		}
-		m_bonds.push_back(std::move(bond));
-	}
-
-	// reconstruct a formula and weight
-
-	m_formula_weight = 0;
-
-	std::map<atom_type, int> f;
-	for (auto &atom : m_atoms)
-		f[atom.type_symbol] += 1;
-	
-	if (f.count(atom_type::C))
-	{
-		atom_type_traits att(atom_type::C);
-		m_formula += att.symbol() + std::to_string(f[atom_type::C]) + ' ';
-		m_formula_weight += att.weight() * f[atom_type::C];
-	}
-
-	for (const auto &[type, count] : f)
-	{
-		if (type == atom_type::C)
-			continue;
-
-		atom_type_traits att(type);
-		m_formula += att.symbol() + std::to_string(count) + ' ';
-		m_formula_weight += att.weight() * count;
-	}
-	
-	if (not m_formula.empty())
-		m_formula.pop_back();
-}
-
 compound_atom compound::get_atom_by_atom_id(const std::string &atom_id) const
 {
 	compound_atom result = {};
@@ -379,8 +296,6 @@ class compound_factory_impl : public std::enable_shared_from_this<compound_facto
 
 	compound *get(std::string id)
 	{
-		cif::to_upper(id);
-
 		std::shared_lock lock(mMutex);
 
 		compound *result = nullptr;
@@ -401,7 +316,8 @@ class compound_factory_impl : public std::enable_shared_from_this<compound_facto
 				break;
 		}
 
-		if (result == nullptr and m_missing.count(id) == 0)
+		if (result == nullptr and
+			find_if(m_missing.begin(), m_missing.end(), [&id](const std::string &m_id) { return cif::iequals(id, m_id); }) == m_missing.end())
 		{
 			for (auto impl = shared_from_this(); impl; impl = impl->m_next)
 			{
@@ -411,7 +327,7 @@ class compound_factory_impl : public std::enable_shared_from_this<compound_facto
 			}
 
 			if (result == nullptr)
-				m_missing.insert(id);
+				m_missing.emplace_back(id);
 		}
 
 		return result;
@@ -444,7 +360,7 @@ class compound_factory_impl : public std::enable_shared_from_this<compound_facto
 	cif::parser::datablock_index m_index;
 
 	std::vector<compound *> m_compounds;
-	std::set<std::string> m_missing;
+	std::vector<std::string> m_missing;
 	std::shared_ptr<compound_factory_impl> m_next;
 };
 
@@ -576,7 +492,7 @@ compound *local_compound_factory_impl::create(const std::string &id)
 
 	for (auto &db : m_local_file)
 	{
-		if (db.name() == "comp_" + id)
+		if (db.name() == id)
 		{
 			auto chem_comp = db.get("chem_comp");
 			if (not chem_comp)

src/datablock.cpp

@@ -38,6 +38,35 @@ datablock::datablock(const datablock &db)
 		cat.update_links(*this);
 }
 
+void datablock::load_dictionary()
+{
+	if (auto *audit_conform = get("audit_conform"); audit_conform and not audit_conform->empty())
+	{
+		std::string name = audit_conform->front().get<std::string>("dict_name");
+
+		if (name == "mmcif_pdbx_v50")
+			name = "mmcif_pdbx.dic"; // we had a bug here in libcifpp...
+
+		if (not name.empty())
+		{
+			try
+			{
+				load_dictionary(name);
+			}
+			catch (const std::exception &ex)
+			{
+				if (VERBOSE)
+					std::cerr << "Failed to load dictionary " << std::quoted(name) << ": " << ex.what() << '\n';
+			}
+		}
+	}
+}
+
+void datablock::load_dictionary(std::string_view name)
+{
+	set_validator(&validator_factory::instance()[name]);
+}
+
 void datablock::set_validator(const validator *v)
 {
 	m_validator = v;
@@ -62,7 +91,7 @@ const validator *datablock::get_validator() const
 bool datablock::is_valid() const
 {
 	if (m_validator == nullptr)
-		throw std::runtime_error("Validator not specified");
+		throw std::runtime_error("Validator not specified for datablock data_" + name());
 
 	bool result = true;
 	for (auto &cat : *this)
@@ -74,7 +103,7 @@ bool datablock::is_valid() const
 bool datablock::is_valid()
 {
 	if (m_validator == nullptr)
-		throw std::runtime_error("Validator not specified");
+		throw std::runtime_error("Validator not specified for datablock data_" + name());
 
 	bool result = true;
 	for (auto &cat : *this)

src/file.cpp

@@ -30,40 +30,8 @@
 namespace cif
 {
 
-// --------------------------------------------------------------------
-// TODO: This is wrong. A validator should be assigned to datablocks,
-// not to a file. Since audit_conform is a category specifying the
-// content of a datablock. Not the entire file.
-
-void file::set_validator(const validator *v)
-{
-	m_validator = v;
-	for (bool first = true; auto &db : *this)
-	{
-		try
-		{
-			db.set_validator(v);
-		}
-		catch (const std::exception &e)
-		{
-			if (first)
-				throw;
-
-			// Accept failure on secondary datablocks
-			// now that many mmCIF files have invalid
-			// restraint data concatenated.
-			std::cerr << e.what() << '\n';
-		}
-
-		first = false;
-	}
-}
-
 bool file::is_valid() const
 {
-	if (m_validator == nullptr)
-		std::runtime_error("No validator loaded explicitly, cannot continue");
-
 	bool result = true;
 	for (auto &d : *this)
 		result = d.is_valid() and result;
@@ -76,14 +44,6 @@ bool file::is_valid() const
 
 bool file::is_valid()
 {
-	if (m_validator == nullptr)
-	{
-		if (VERBOSE > 0)
-			std::cerr << "No dictionary loaded explicitly, loading default\n";
-
-		load_dictionary();
-	}
-
 	bool result = not empty();
 
 	for (auto &d : *this)
@@ -97,9 +57,6 @@ bool file::is_valid()
 
 bool file::validate_links() const
 {
-	if (m_validator == nullptr)
-		std::runtime_error("No validator loaded explicitly, cannot continue");
-
 	bool result = true;
 
 	for (auto &db : *this)
@@ -108,41 +65,41 @@ bool file::validate_links() const
 	return result;
 }
 
-void file::load_dictionary()
-{
-	if (not empty())
-	{
-		auto *audit_conform = front().get("audit_conform");
-		if (audit_conform and not audit_conform->empty())
-		{
-			std::string name = audit_conform->front().get<std::string>("dict_name");
+// void file::load_dictionary()
+// {
+// 	if (not empty())
+// 	{
+// 		auto *audit_conform = front().get("audit_conform");
+// 		if (audit_conform and not audit_conform->empty())
+// 		{
+// 			std::string name = audit_conform->front().get<std::string>("dict_name");
 
-			if (name == "mmcif_pdbx_v50")
-				name = "mmcif_pdbx.dic"; // we had a bug here in libcifpp...
+// 			if (name == "mmcif_pdbx_v50")
+// 				name = "mmcif_pdbx.dic"; // we had a bug here in libcifpp...
 
-			if (not name.empty())
-			{
-				try
-				{
-					load_dictionary(name);
-				}
-				catch (const std::exception &ex)
-				{
-					if (VERBOSE)
-						std::cerr << "Failed to load dictionary " << std::quoted(name) << ": " << ex.what() << '\n';
-				}
-			}
-		}
-	}
+// 			if (not name.empty())
+// 			{
+// 				try
+// 				{
+// 					load_dictionary(name);
+// 				}
+// 				catch (const std::exception &ex)
+// 				{
+// 					if (VERBOSE)
+// 						std::cerr << "Failed to load dictionary " << std::quoted(name) << ": " << ex.what() << '\n';
+// 				}
+// 			}
+// 		}
+// 	}
 
-	// if (not m_validator)
-	// 	load_dictionary("mmcif_pdbx.dic");	// TODO: maybe incorrect? Perhaps improve?
-}
+// 	// if (not m_validator)
+// 	// 	load_dictionary("mmcif_pdbx.dic");	// TODO: maybe incorrect? Perhaps improve?
+// }
 
-void file::load_dictionary(std::string_view name)
-{
-	set_validator(&validator_factory::instance()[name]);
-}
+// void file::load_dictionary(std::string_view name)
+// {
+// 	set_validator(&validator_factory::instance()[name]);
+// }
 
 bool file::contains(std::string_view name) const
 {
@@ -187,10 +144,7 @@ std::tuple<file::iterator, bool> file::emplace(std::string_view name)
 	}
 
 	if (is_new)
-	{
 		i = insert(end(), { name });
-		i->set_validator(m_validator);
-	}
 
 	assert(i != end());
 	return std::make_tuple(i, is_new);
@@ -212,18 +166,44 @@ void file::load(const std::filesystem::path &p)
 	}
 }
 
-void file::load(std::istream &is)
+void file::load(const std::filesystem::path &p, std::string_view dict)
 {
-	auto saved = m_validator;
-	set_validator(nullptr);
+	load(p, validator_factory::instance().operator[](dict));
+}
 
+void file::load(std::istream &is, std::string_view dict)
+{
+	load(is, validator_factory::instance().operator[](dict));
+}
+
+void file::load(const std::filesystem::path &p, const validator &v)
+{
+	gzio::ifstream in(p);
+	if (not in.is_open())
+		throw std::runtime_error("Could not open file '" + p.string() + '\'');
+
+	try
+	{
+		load(in, v);
+	}
+	catch (const std::exception &)
+	{
+		throw_with_nested(std::runtime_error("Error reading file '" + p.string() + '\''));
+	}
+}
+
+void file::load(std::istream &is, const validator &v)
+{
 	parser p(is, *this);
 	p.parse_file();
+	for (auto &db : *this)
+		db.set_validator(&v);
+}
 
-	if (saved != nullptr)
-		set_validator(saved);
-	else
-		load_dictionary();
+void file::load(std::istream &is)
+{
+	parser p(is, *this);
+	p.parse_file();
 }
 
 void file::save(const std::filesystem::path &p) const

src/parser.cpp

@@ -837,6 +837,9 @@ void parser::produce_datablock(std::string_view name)
 
 	const auto &[iter, ignore] = m_file.emplace(name);
 	m_datablock = &(*iter);
+
+	if (m_validator)
+		m_datablock->set_validator(m_validator);
 }
 
 void parser::produce_category(std::string_view name)

src/pdb/pdb2cif.cpp

@@ -5909,7 +5909,8 @@ void PDBFileParser::Parse(std::istream &is, cif::file &result)
 {
 	try
 	{
-		mDatablock.set_validator(result.get_validator());
+		if (mDatablock.get_validator() == nullptr)
+			mDatablock.load_dictionary();
 
 		PreParseInput(is);
 
@@ -6373,10 +6374,11 @@ void read_pdb_file(std::istream &pdbFile, cif::file &cifFile)
 {
 	PDBFileParser p;
 
-	cifFile.load_dictionary("mmcif_pdbx.dic");
-
 	p.Parse(pdbFile, cifFile);
 
+	if (not cifFile.empty() and cifFile.front().get_validator() == nullptr)
+		cifFile.front().load_dictionary("mmcif_pdbx.dic");
+
 	if (not cifFile.is_valid() and cif::VERBOSE >= 0)
 		std::cerr << "Resulting mmCIF file is not valid!\n";
 }
@@ -6421,8 +6423,8 @@ file read(std::istream &is)
 	}
 
 	// Must be a PDB like file, right?
-	if (result.get_validator() == nullptr)
-		result.load_dictionary("mmcif_pdbx.dic");
+	if (not result.empty() and result.front().get_validator() == nullptr)
+		result.front().load_dictionary("mmcif_pdbx.dic");
 
 	return result;
 }

src/pdb/pdb2cif_remark_3.cpp

@@ -1480,6 +1480,9 @@ bool Remark3Parser::parse(const std::string &expMethod, PDBRecord *r, cif::datab
 
 		for (auto &cat1 : best.parser->mDb)
 		{
+			if (cat1.empty())
+				continue;
+
 			auto &cat2 = db[cat1.name()];
 
 			// copy only the values in the first row for the following categories

src/pdb/reconstruct.cpp

@@ -100,10 +100,35 @@ void checkEntities(datablock &db)
 
 	for (auto entity : db["entity"].find("formula_weight"_key == null or "formula_weight"_key == 0))
 	{
-		const auto &[entity_id, type] = entity.get<std::string, std::string>("id", "type");
+		auto &&[entity_id, type] = entity.get<std::string, std::string>("id", "type");
 
 		float formula_weight = 0;
 
+		if (type.empty())	// yes, that happens
+		{
+			const auto comp_id = db["atom_site"].find_first<std::string>("label_entity_id"_key == entity_id, "label_comp_id");
+			auto compound = cf.create(comp_id);
+			if (compound != nullptr)
+			{
+				if (compound->is_base() or compound->is_peptide())
+					type = "polymer";
+				else if (compound->is_water())
+					type = "water";
+				else
+				{
+					if (db["pdbx_entity_branch_link"].contains("entity_id"_key == entity_id))
+						type = "branched";
+					else
+						type = "non-polymer";
+				}
+			}
+
+			if (type.empty())
+				throw std::runtime_error("Entity without type and cannot determine what it should be");
+			
+			entity["type"] = type;
+		}
+		
 		if (type == "polymer")
 		{
 			int n = 0;
@@ -1012,6 +1037,10 @@ void comparePolySeqSchemes(datablock &db)
 	auto &ndb_poly_seq_scheme = db["ndb_poly_seq_scheme"];
 	auto &pdbx_poly_seq_scheme = db["pdbx_poly_seq_scheme"];
 
+	// Don't bother if ndb_poly_seq_scheme is empty
+	if (ndb_poly_seq_scheme.empty())
+		return;
+
 	// Since often ndb_poly_seq_scheme only contains an id and mon_id item
 	// we assume that it should match the accompanying pdbx_poly_seq
 
@@ -1080,6 +1109,87 @@ void comparePolySeqSchemes(datablock &db)
 	}
 }
 
+void createPdbxEntityNonpoly(datablock &db)
+{
+	using namespace literals;
+
+	auto &atom_site = db["atom_site"];
+	auto &entity = db["entity"];
+	auto &pdbx_entity_nonpoly = db["pdbx_entity_nonpoly"];
+
+	for (const auto &[entity_id, type] : entity.find<std::string, std::string>("type"_key == "water" or "type"_key == "non-polymer", "id", "type"))
+	{
+		for (auto comp_id : atom_site.find<std::string>("label_entity_id"_key == entity_id, "label_comp_id"))
+		{
+			if (auto test_comp_id = pdbx_entity_nonpoly.find_first<std::optional<std::string>>("entity_id"_key == entity_id, "comp_id"); test_comp_id.has_value())
+			{
+				if (*test_comp_id != comp_id)
+					throw std::runtime_error("Inconsistent pdbx_entity_nonpoly record for entity " + entity_id + ", expected comp_id " + comp_id);
+
+				continue;
+			}
+
+			if (type == "water")
+				pdbx_entity_nonpoly.emplace({ //
+					{ "entity_id", entity_id },
+					{ "name", "water" },
+					{ "comp_id", comp_id }
+				});
+			else
+			{
+				auto c = cif::compound_factory::instance().create(comp_id);
+
+				std::string name = c ? c->name() : ".";
+
+				pdbx_entity_nonpoly.emplace({ //
+					{ "entity_id", entity_id },
+					{ "name", name },
+					{ "comp_id", comp_id }
+				});
+			}
+		}
+	}
+}
+
+void createPdbxNonpolyScheme(datablock &db)
+{
+	using namespace literals;
+
+	createPdbxEntityNonpoly(db);
+
+	auto &pdbx_entity_nonpoly = db["pdbx_entity_nonpoly"];
+	auto &pdbx_nonpoly_scheme = db["pdbx_nonpoly_scheme"];
+	auto &atom_site = db["atom_site"];
+
+	for (const auto &[entity_id, comp_id] : pdbx_entity_nonpoly.rows<std::string,std::string>("entity_id", "comp_id"))
+	{
+		for (int ndb_nr = 1; auto row : atom_site.find("label_entity_id"_key == entity_id and "label_comp_id"_key == comp_id))
+		{
+			// Skip existing records
+			auto linked = atom_site.get_linked(row, pdbx_nonpoly_scheme);
+			if (not linked.empty())
+				continue;
+
+			int num = row.get<int>("auth_seq_id");
+
+			pdbx_nonpoly_scheme.emplace({//
+
+				{ "asym_id", row.get<std::string>("label_asym_id") },
+				{ "entity_id", entity_id },
+				{ "mon_id", comp_id },
+				{ "ndb_seq_num", ndb_nr++ },
+				{ "pdb_seq_num", num },
+				{ "auth_seq_num", num },
+				{ "pdb_mon_id", row.get<std::string>("auth_comp_id") },
+				{ "auth_mon_id",  row.get<std::string>("auth_comp_id") },
+				{ "pdb_strand_id", row.get<std::string>("auth_asym_id") },
+				{ "pdb_ins_code", row.get<std::string>("pdbx_PDB_ins_code") }
+
+			});
+		}
+	}
+}
+
 bool reconstruct_pdbx(file &file, std::string_view dictionary)
 {
 	if (file.empty())
@@ -1346,7 +1456,7 @@ bool reconstruct_pdbx(file &file, std::string_view dictionary)
 
 	db["chem_comp"].reorder_by_index();
 
-	file.load_dictionary(dictionary);
+	db.load_dictionary(dictionary);
 
 	if (db.get("atom_site_anisotrop"))
 		checkAtomAnisotropRecords(db);
@@ -1367,6 +1477,8 @@ bool reconstruct_pdbx(file &file, std::string_view dictionary)
 
 	if (auto cat = db.get("ndb_poly_seq_scheme"); cat == nullptr or cat->empty())
 		comparePolySeqSchemes(db);
+	
+	createPdbxNonpolyScheme(db);
 
 	// skip unknown categories for now
 	bool valid = true;

src/pdb/validate-pdbx.cpp

@@ -306,8 +306,8 @@ bool is_valid_pdbx_file(const file &file, std::string_view dictionary, std::erro
 
 			if (not seq_can.has_value())
 			{
-				if (cif::VERBOSE > 0)
-					std::clog << "Warning: entity_poly has no sequence for entity_id " << entity_id << '\n';
+				if (cif::VERBOSE > 1)
+					std::clog << "Warning: entity_poly has no canonical sequence for entity_id " << entity_id << '\n';
 			}
 			else
 			{

src/symmetry.cpp

@@ -32,7 +32,7 @@
 
 #include "symop_table_data.hpp"
 
-#include <Eigen/Eigenvalues>
+#include <Eigen/Eigen>
 
 namespace cif
 {
@@ -103,9 +103,9 @@ sym_op::sym_op(std::string_view s)
 	auto b = s.data();
 	auto e = b + s.length();
 
-	int rnri = 256;	// default to unexisting number
+	int rnri = 256; // default to unexisting number
 	auto r = std::from_chars(b, e, rnri);
-	
+
 	m_nr = static_cast<uint8_t>(rnri);
 	m_ta = r.ptr[1] - '0';
 	m_tb = r.ptr[2] - '0';
@@ -121,7 +121,7 @@ std::string sym_op::string() const
 	auto r = std::to_chars(b, b + sizeof(b), m_nr);
 	if ((bool)r.ec or r.ptr > b + 4)
 		throw std::runtime_error("Could not write out symmetry operation to string");
-	
+
 	*r.ptr++ = '_';
 	*r.ptr++ = '0' + m_ta;
 	*r.ptr++ = '0' + m_tb;
@@ -163,41 +163,16 @@ transformation::transformation(const matrix3x3<float> &r, const cif::point &t)
 
 void transformation::try_create_quaternion()
 {
-	float Qxx = m_rotation(0, 0);
-	float Qxy = m_rotation(0, 1);
-	float Qxz = m_rotation(0, 2);
-	float Qyx = m_rotation(1, 0);
-	float Qyy = m_rotation(1, 1);
-	float Qyz = m_rotation(1, 2);
-	float Qzx = m_rotation(2, 0);
-	float Qzy = m_rotation(2, 1);
-	float Qzz = m_rotation(2, 2);
+	Eigen::Matrix3f rot;
 
-	Eigen::Matrix4f em;
+	rot << m_rotation(0, 0), m_rotation(0, 1), m_rotation(0, 2),
+		m_rotation(1, 0), m_rotation(1, 1), m_rotation(1, 2),
+		m_rotation(2, 0), m_rotation(2, 1), m_rotation(2, 2);
 
-	em << Qxx - Qyy - Qzz, Qyx + Qxy, Qzx + Qxz, Qzy - Qyz,
-			Qyx + Qxy, Qyy - Qxx - Qzz, Qzy + Qyz, Qxz - Qzx,
-			Qzx + Qxz, Qzy + Qyz, Qzz - Qxx - Qyy, Qyx - Qxy,
-			Qzy - Qyz, Qxz - Qzx, Qyx - Qxy, Qxx + Qyy + Qzz;
-
-	Eigen::EigenSolver<Eigen::Matrix4f> es(em / 3);
-
-	auto ev = es.eigenvalues();
-
-	for (std::size_t j = 0; j < 4; ++j)
+	if (rot * rot.transpose() == Eigen::Matrix3f::Identity() and rot.determinant() == 1)
 	{
-		if (std::abs(ev[j].real() - 1) > 0.01)
-			continue;
-		
-		auto col = es.eigenvectors().col(j);
-
-		m_q = normalize(cif::quaternion{
-			static_cast<float>(col(3).real()),
-			static_cast<float>(col(0).real()),
-			static_cast<float>(col(1).real()),
-			static_cast<float>(col(2).real()) });
-
-		break;
+		Eigen::Quaternionf qe(rot);
+		m_q = normalize(cif::quaternion{ qe.w(), qe.x(), qe.y(), qe.z() });
 	}
 }
 
@@ -297,7 +272,7 @@ point spacegroup::operator()(const point &pt, const cell &c, sym_op symop) const
 {
 	if (symop.m_nr < 1 or symop.m_nr > size())
 		throw std::out_of_range("symmetry operator number out of range");
-	
+
 	transformation t = at(symop.m_nr - 1);
 
 	t.m_translation.m_x += symop.m_ta - 5;
@@ -316,7 +291,7 @@ point spacegroup::inverse(const point &pt, const cell &c, sym_op symop) const
 {
 	if (symop.m_nr < 1 or symop.m_nr > size())
 		throw std::out_of_range("symmetry operator number out of range");
-	
+
 	transformation t = at(symop.m_nr - 1);
 
 	t.m_translation.m_x += symop.m_ta - 5;
@@ -450,13 +425,13 @@ int get_space_group_number(const datablock &db)
 
 	if (_symmetry.size() != 1)
 		throw std::runtime_error("Could not find a unique symmetry in this mmCIF file");
-	
+
 	return _symmetry.front().get<int>("Int_Tables_number");
 }
 
 // --------------------------------------------------------------------
 
-std::tuple<float,point,sym_op> crystal::closest_symmetry_copy(point a, point b) const
+std::tuple<float, point, sym_op> crystal::closest_symmetry_copy(point a, point b) const
 {
 	if (m_cell.get_a() == 0 or m_cell.get_b() == 0 or m_cell.get_c() == 0)
 		throw std::runtime_error("Invalid cell, contains a dimension that is zero");
@@ -491,7 +466,7 @@ std::tuple<float,point,sym_op> crystal::closest_symmetry_copy(point a, point b)
 		while (fsb.m_x + 0.5f < fa.m_x)
 		{
 			fsb.m_x += 1;
-			s.m_ta += 1;			
+			s.m_ta += 1;
 		}
 
 		while (fsb.m_y - 0.5f > fa.m_y)
@@ -503,7 +478,7 @@ std::tuple<float,point,sym_op> crystal::closest_symmetry_copy(point a, point b)
 		while (fsb.m_y + 0.5f < fa.m_y)
 		{
 			fsb.m_y += 1;
-			s.m_tb += 1;			
+			s.m_tb += 1;
 		}
 
 		while (fsb.m_z - 0.5f > fa.m_z)
@@ -515,7 +490,7 @@ std::tuple<float,point,sym_op> crystal::closest_symmetry_copy(point a, point b)
 		while (fsb.m_z + 0.5f < fa.m_z)
 		{
 			fsb.m_z += 1;
-			s.m_tc += 1;			
+			s.m_tc += 1;
 		}
 
 		auto p = orthogonal(fsb, m_cell);

src/text.cpp

@@ -69,7 +69,7 @@ bool iequals(const char *a, const char *b)
 {
 	bool result = true;
 	for (; result and *a and *b; ++a, ++b)
-		result = tolower(*a) == tolower(*b);
+		result = kCharToLowerMap[uint8_t(*a)] == kCharToLowerMap[uint8_t(*b)];
 
 	return result and *a == *b;
 }
@@ -80,7 +80,7 @@ int icompare(std::string_view a, std::string_view b)
 	auto ai = a.begin(), bi = b.begin();
 
 	for (; d == 0 and ai != a.end() and bi != b.end(); ++ai, ++bi)
-		d = tolower(*ai) - tolower(*bi);
+		d = (int)kCharToLowerMap[uint8_t(*ai)] - (int)kCharToLowerMap[uint8_t(*bi)];
 
 	if (d == 0)
 	{
@@ -98,7 +98,7 @@ int icompare(const char *a, const char *b)
 	int d = 0;
 
 	for (; d == 0 and *a != 0 and *b != 0; ++a, ++b)
-		d = tolower(*a) - tolower(*b);
+		d = (int)kCharToLowerMap[uint8_t(*a)] - (int)kCharToLowerMap[uint8_t(*b)];
 
 	if (d == 0)
 	{

src/utilities.cpp

@@ -63,9 +63,9 @@ std::string get_version_nr()
 
 // --------------------------------------------------------------------
 
-#ifdef _WIN32
+#if defined(_WIN32) or defined(__MINGW32__)
 }
-#include <Windows.h>
+#include <windows.h>
 #include <libloaderapi.h>
 #include <wincon.h>
 

src/validate.cpp

@@ -138,7 +138,7 @@ int type_validator::compare(std::string_view a, std::string_view b) const
 				ra = selected_charconv<double>::from_chars(a.data(), a.data() + a.length(), da);
 				rb = selected_charconv<double>::from_chars(b.data(), b.data() + b.length(), db);
 
-				if (not (bool)ra.ec and not (bool)rb.ec)
+				if (not(bool) ra.ec and not(bool) rb.ec)
 				{
 					auto d = da - db;
 					if (std::abs(d) > std::numeric_limits<double>::epsilon())
@@ -232,7 +232,7 @@ bool item_validator::validate_value(std::string_view value, std::error_code &ec)
 			ec = make_error_code(validation_error::value_is_not_in_enumeration_list);
 	}
 
-	return not (bool)ec;
+	return not(bool) ec;
 }
 
 // --------------------------------------------------------------------
@@ -401,20 +401,18 @@ void validator::report_error(std::error_code ec, bool fatal) const
 {
 	if (m_strict or fatal)
 		throw validation_exception(ec);
-	else
+	else if (VERBOSE > 0)
 		std::cerr << ec.message() << '\n';
 }
 
 void validator::report_error(std::error_code ec, std::string_view category,
 	std::string_view item, bool fatal) const
 {
-	auto ex = item.empty() ?
-		validation_exception(ec, category) :
-		validation_exception(ec, category, item);
+	auto ex = item.empty() ? validation_exception(ec, category) : validation_exception(ec, category, item);
 
 	if (m_strict or fatal)
 		throw ex;
-	else
+	else if (VERBOSE > 0)
 		std::cerr << ex.what() << '\n';
 }
 
@@ -455,59 +453,71 @@ const validator &validator_factory::operator[](std::string_view dictionary_name)
 		}
 
 		// not found, add it
-		auto data = load_resource(dictionary_name);
 
-		if (not data and dictionary.extension().string() != ".dic")
-			data = load_resource(dictionary.parent_path() / (dictionary.filename().string() + ".dic"));
+		validator v(dictionary_name);
 
-		if (data)
-			construct_validator(dictionary_name, *data);
-		else
+		for (bool first = true; auto part_name : cif::split(dictionary_name, ";", true))
 		{
-			std::error_code ec;
+			auto data = load_resource(part_name);
+			dictionary.assign(part_name.begin(), part_name.end());
 
-			// might be a compressed dictionary on disk
-			std::filesystem::path p = dictionary;
-			if (p.extension() == ".dic")
-				p = p.parent_path() / (p.filename().string() + ".gz");
-			else
-				p = p.parent_path() / (p.filename().string() + ".dic.gz");
+			if (not data and dictionary.extension().string() != ".dic")
+				data = load_resource(dictionary.parent_path() / (dictionary.filename().string() + ".dic"));
+
+			if (not data)
+			{
+				std::error_code ec;
+
+				// might be a compressed dictionary on disk
+				std::filesystem::path p = dictionary;
+				if (p.extension() == ".dic")
+					p = p.parent_path() / (p.filename().string() + ".gz");
+				else
+					p = p.parent_path() / (p.filename().string() + ".dic.gz");
 
 #if defined(CACHE_DIR) or defined(DATA_DIR)
-			if (not std::filesystem::exists(p, ec) or ec)
-			{
-				for (const char *dir : {
+				if (not std::filesystem::exists(p, ec) or ec)
+				{
+					for (const char *dir : {
 # if defined(CACHE_DIR)
-						 CACHE_DIR,
+							 CACHE_DIR,
 # endif
 # if defined(DATA_DIR)
 							 DATA_DIR
 # endif
-					 })
-				{
-					auto p2 = std::filesystem::path(dir) / p;
-					if (std::filesystem::exists(p2, ec) and not ec)
+						 })
 					{
-						swap(p, p2);
-						break;
+						auto p2 = std::filesystem::path(dir) / p;
+						if (std::filesystem::exists(p2, ec) and not ec)
+						{
+							swap(p, p2);
+							break;
+						}
 					}
 				}
-			}
 #endif
 
-			if (std::filesystem::exists(p, ec) and not ec)
-			{
-				gzio::ifstream in(p);
+				if (std::filesystem::exists(p, ec) and not ec)
+				{
+					auto in = std::make_unique<gzio::ifstream>(p);
 
-				if (not in.is_open())
-					throw std::runtime_error("Could not open dictionary (" + p.string() + ")");
+					if (not in->is_open())
+						throw std::runtime_error("Could not open dictionary (" + p.string() + ")");
 
-				construct_validator(dictionary_name, in);
+					data.reset(in.release());
+				}
+				else
+					throw std::runtime_error("Dictionary not found or defined (" + dictionary.string() + ")");
 			}
+
+			if (std::exchange(first, false))
+				v = parse_dictionary(part_name, *data);
 			else
-				throw std::runtime_error("Dictionary not found or defined (" + dictionary.string() + ")");
+				extend_dictionary(v, *data);
 		}
 
+		m_validators.emplace_back(std::move(v));
+
 		return m_validators.back();
 	}
 	catch (const std::exception &ex)

test/CMakeLists.txt

@@ -34,10 +34,10 @@ add_library(test-main OBJECT "${CMAKE_CURRENT_SOURCE_DIR}/test-main.cpp")
 
 target_link_libraries(test-main cifpp::cifpp Catch2::Catch2)
 
-if(${Catch2_VERSION} VERSION_GREATER_EQUAL 3.0.0)
-	target_compile_definitions(test-main PUBLIC CATCH22=0)
-else()
+if("${Catch2_VERSION}" VERSION_LESS 3.0.0)
 	target_compile_definitions(test-main PUBLIC CATCH22=1)
+else()
+	target_compile_definitions(test-main PUBLIC CATCH22=0)
 endif()
 
 foreach(CIFPP_TEST IN LISTS CIFPP_tests)

test/model-test.cpp

@@ -53,8 +53,8 @@ TEST_CASE("create_nonpoly_1")
 	cif::VERBOSE = 1;
 
 	cif::file file;
-	file.load_dictionary("mmcif_pdbx.dic");
-	file.emplace("TEST"); // create a datablock
+	auto &&[dbi, ignore] = file.emplace("TEST"); // create a datablock
+	dbi->load_dictionary("mmcif_pdbx.dic");
 
 	cif::mm::structure structure(file);
 
@@ -82,7 +82,7 @@ _atom_site.pdbx_formal_charge
 # that's enough to test with
 )"_cf;
 
-	atoms.load_dictionary("mmcif_pdbx.dic");
+	atoms.front().load_dictionary("mmcif_pdbx.dic");
 
 	auto &hem_data = atoms["HEM"];
 	auto &atom_site = hem_data["atom_site"];
@@ -159,7 +159,7 @@ _struct_asym.details                       ?
 _atom_type.symbol   C
 )"_cf;
 
-	expected.load_dictionary("mmcif_pdbx.dic");
+	expected.front().load_dictionary("mmcif_pdbx.dic");
 
 	if (not(expected.front() == structure.get_datablock()))
 	{
@@ -177,8 +177,8 @@ TEST_CASE("create_nonpoly_2")
 	cif::VERBOSE = 1;
 
 	cif::file file;
-	file.load_dictionary("mmcif_pdbx.dic");
-	file.emplace("TEST"); // create a datablock
+	auto &&[dbi, ignore] = file.emplace("TEST"); // create a datablock
+	dbi->load_dictionary("mmcif_pdbx.dic");
 
 	cif::mm::structure structure(file);
 
@@ -270,7 +270,7 @@ _struct_asym.details                       ?
 _atom_type.symbol   C
 )"_cf;
 
-	expected.load_dictionary("mmcif_pdbx.dic");
+	expected.front().load_dictionary("mmcif_pdbx.dic");
 
 	REQUIRE(expected.front() == structure.get_datablock());
 
@@ -354,7 +354,7 @@ _struct_asym.details                       ?
 #
 )"_cf;
 
-	data.load_dictionary("mmcif_pdbx.dic");
+	data.front().load_dictionary("mmcif_pdbx.dic");
 
 	cif::mm::structure s(data);
 

test/unit-3d-test.cpp

@@ -148,9 +148,8 @@ TEST_CASE("dh_q_0")
 	cif::point axis(1, 0, 0);
 
 	cif::point p(1, 1, 0);
-	
-	cif::point t[3] =
-	{
+
+	cif::point t[3] = {
 		{ 0, 1, 0 },
 		{ 0, 0, 0 },
 		{ 1, 0, 0 }
@@ -180,7 +179,6 @@ TEST_CASE("dh_q_0")
 
 	a = cif::dihedral_angle(t[0], t[1], t[2], p);
 	REQUIRE(std::abs(a - 0.f) < 0.01f);
-
 }
 
 TEST_CASE("dh_q_1")
@@ -228,62 +226,103 @@ TEST_CASE("dh_q_1")
 
 // --------------------------------------------------------------------
 
-TEST_CASE("m2q_0, *utf::tolerance(0.001f)")
+// TEST_CASE("m2q_0")
+// {
+// 	for (std::size_t i = 0; i < cif::kSymopNrTableSize; ++i)
+// 	{
+// 		auto d = cif::kSymopNrTable[i].symop().data();
+
+// 		cif::matrix3x3<float> rot;
+// 		float Qxx = rot(0, 0) = d[0];
+// 		float Qxy = rot(0, 1) = d[1];
+// 		float Qxz = rot(0, 2) = d[2];
+// 		float Qyx = rot(1, 0) = d[3];
+// 		float Qyy = rot(1, 1) = d[4];
+// 		float Qyz = rot(1, 2) = d[5];
+// 		float Qzx = rot(2, 0) = d[6];
+// 		float Qzy = rot(2, 1) = d[7];
+// 		float Qzz = rot(2, 2) = d[8];
+
+// 		Eigen::Matrix4f em;
+
+// 		em << Qxx - Qyy - Qzz, Qyx + Qxy, Qzx + Qxz, Qzy - Qyz,
+// 			Qyx + Qxy, Qyy - Qxx - Qzz, Qzy + Qyz, Qxz - Qzx,
+// 			Qzx + Qxz, Qzy + Qyz, Qzz - Qxx - Qyy, Qyx - Qxy,
+// 			Qzy - Qyz, Qxz - Qzx, Qyx - Qxy, Qxx + Qyy + Qzz;
+
+// 		Eigen::EigenSolver<Eigen::Matrix4f> es(em / 3);
+
+// 		auto ev = es.eigenvalues();
+
+// 		std::size_t bestJ = 0;
+// 		float bestEV = -1;
+
+// 		for (std::size_t j = 0; j < 4; ++j)
+// 		{
+// 			if (bestEV < ev[j].real())
+// 			{
+// 				bestEV = ev[j].real();
+// 				bestJ = j;
+// 			}
+// 		}
+
+// 		if (std::abs(bestEV - 1) > 0.01)
+// 			continue; // not a rotation matrix
+
+// 		auto col = es.eigenvectors().col(bestJ);
+
+// 		auto q = normalize(cif::quaternion{
+// 			static_cast<float>(col(3).real()),
+// 			static_cast<float>(col(0).real()),
+// 			static_cast<float>(col(1).real()),
+// 			static_cast<float>(col(2).real()) });
+
+// 		cif::point p1{ 1, 1, 1 };
+// 		cif::point p2 = p1;
+// 		p2.rotate(q);
+
+// 		cif::point p3 = rot * p1;
+
+// 		REQUIRE_THAT(p2.m_x, Catch::Matchers::WithinRel(p3.m_x, 0.01f));
+// 		REQUIRE_THAT(p2.m_y, Catch::Matchers::WithinRel(p3.m_y, 0.01f));
+// 		REQUIRE_THAT(p2.m_z, Catch::Matchers::WithinRel(p3.m_z, 0.01f));
+// 	}
+// }
+
+TEST_CASE("m2q_0a")
 {
 	for (std::size_t i = 0; i < cif::kSymopNrTableSize; ++i)
 	{
 		auto d = cif::kSymopNrTable[i].symop().data();
 
-		cif::matrix3x3<float> rot;
-		float Qxx = rot(0, 0) = d[0];
-		float Qxy = rot(0, 1) = d[1];
-		float Qxz = rot(0, 2) = d[2];
-		float Qyx = rot(1, 0) = d[3];
-		float Qyy = rot(1, 1) = d[4];
-		float Qyz = rot(1, 2) = d[5];
-		float Qzx = rot(2, 0) = d[6];
-		float Qzy = rot(2, 1) = d[7];
-		float Qzz = rot(2, 2) = d[8];
+		Eigen::Matrix3f rot;
+		rot << d[0], d[1], d[2], d[3], d[4], d[5], d[6], d[7], d[8];
 
-		Eigen::Matrix4f em;
+		// check to see if this matrix contains a true rotation
+		if (rot * rot.transpose() != Eigen::Matrix3f::Identity() or rot.determinant() != 1)
+			continue;
 
-		em << Qxx - Qyy - Qzz, Qyx + Qxy, Qzx + Qxz, Qzy - Qyz,
-		      Qyx + Qxy, Qyy - Qxx - Qzz, Qzy + Qyz, Qxz - Qzx,
-			  Qzx + Qxz, Qzy + Qyz, Qzz - Qxx - Qyy, Qyx - Qxy,
-			  Qzy - Qyz, Qxz - Qzx, Qyx - Qxy, Qxx + Qyy + Qzz;
+		Eigen::Quaternionf qe(rot);
 
-		Eigen::EigenSolver<Eigen::Matrix4f> es(em / 3);
+		auto q = normalize(cif::quaternion{ qe.w(), qe.x(), qe.y(), qe.z() });
 
-		auto ev = es.eigenvalues();
-
-		std::size_t bestJ = 0;
-		float bestEV = -1;
-
-		for (std::size_t j = 0; j < 4; ++j)
-		{
-			if (bestEV < ev[j].real())
-			{
-				bestEV = ev[j].real();
-				bestJ = j;
-			}
-		}
-
-		if (std::abs(bestEV - 1) > 0.01)
-			continue; // not a rotation matrix
-
-		auto col = es.eigenvectors().col(bestJ);
-
-		auto q = normalize(cif::quaternion{
-			static_cast<float>(col(3).real()),
-			static_cast<float>(col(0).real()),
-			static_cast<float>(col(1).real()),
-			static_cast<float>(col(2).real()) });
-		
 		cif::point p1{ 1, 1, 1 };
 		cif::point p2 = p1;
 		p2.rotate(q);
 
-		cif::point p3 = rot * p1;
+		cif::matrix3x3<float> rot_c({
+			rot_c(0, 0) = d[0],
+			rot_c(0, 1) = d[1],
+			rot_c(0, 2) = d[2],
+			rot_c(1, 0) = d[3],
+			rot_c(1, 1) = d[4],
+			rot_c(1, 2) = d[5],
+			rot_c(2, 0) = d[6],
+			rot_c(2, 1) = d[7],
+			rot_c(2, 2) = d[8]
+		});
+
+		cif::point p3 = rot_c * p1;
 
 		REQUIRE_THAT(p2.m_x, Catch::Matchers::WithinRel(p3.m_x, 0.01f));
 		REQUIRE_THAT(p2.m_y, Catch::Matchers::WithinRel(p3.m_y, 0.01f));
@@ -291,7 +330,7 @@ TEST_CASE("m2q_0, *utf::tolerance(0.001f)")
 	}
 }
 
-// "TEST_CASE(m2q_1, *utf::tolerance(0.001f)")
+// "TEST_CASE(m2q_1")
 // {
 // 	for (std::size_t i = 0; i < cif::kSymopNrTableSize; ++i)
 // 	{
@@ -337,7 +376,7 @@ TEST_CASE("m2q_0, *utf::tolerance(0.001f)")
 // 			static_cast<float>(em(bestJ, 0)),
 // 			static_cast<float>(em(bestJ, 1)),
 // 			static_cast<float>(em(bestJ, 2)) });
-		
+
 // 		cif::point p1{ 1, 1, 1 };
 // 		cif::point p2 = p1;
 // 		p2.rotate(q);
@@ -390,17 +429,17 @@ TEST_CASE("symm_3")
 	REQUIRE(sg.get_name() == "P 21 21 2");
 }
 
-TEST_CASE("symm_4, *utf::tolerance(0.1f)")
+TEST_CASE("symm_4")
 {
 	using namespace cif::literals;
 
 	// based on 2b8h
 	auto sg = cif::spacegroup(154); // p 32 2 1
 	auto c = cif::cell(107.516, 107.516, 338.487, 90.00, 90.00, 120.00);
-	
-	cif::point a{   -8.688,  79.351, 10.439 }; // O6 NAG A 500
-	cif::point b{  -35.356,  33.693, -3.236 }; // CG2 THR D 400
-	cif::point sb(  -6.916,   79.34,   3.236); // 4_565 copy of b
+
+	cif::point a{ -8.688, 79.351, 10.439 };  // O6 NAG A 500
+	cif::point b{ -35.356, 33.693, -3.236 }; // CG2 THR D 400
+	cif::point sb(-6.916, 79.34, 3.236);     // 4_565 copy of b
 
 	REQUIRE_THAT(distance(a, sg(a, c, "1_455"_symop)), Catch::Matchers::WithinRel(static_cast<float>(c.get_a()), 0.01f));
 	REQUIRE_THAT(distance(a, sg(a, c, "1_545"_symop)), Catch::Matchers::WithinRel(static_cast<float>(c.get_b()), 0.01f));
@@ -411,12 +450,12 @@ TEST_CASE("symm_4, *utf::tolerance(0.1f)")
 	REQUIRE_THAT(sb.m_y, Catch::Matchers::WithinRel(sb2.m_y, 0.01f));
 	REQUIRE_THAT(sb.m_z, Catch::Matchers::WithinRel(sb2.m_z, 0.01f));
 
-	REQUIRE_THAT(distance(a, sb2), Catch::Matchers::WithinRel(7.42f, 0.01f));	
+	REQUIRE_THAT(distance(a, sb2), Catch::Matchers::WithinRel(7.42f, 0.01f));
 }
 
 // --------------------------------------------------------------------
 
-TEST_CASE("symm_4wvp_1, *utf::tolerance(0.1f)")
+TEST_CASE("symm_4wvp_1")
 {
 	using namespace cif::literals;
 
@@ -427,7 +466,7 @@ TEST_CASE("symm_4wvp_1, *utf::tolerance(0.1f)")
 
 	cif::crystal c(db);
 
-	cif::point p{ -78.722, 98.528,  11.994 };
+	cif::point p{ -78.722, 98.528, 11.994 };
 	auto a = s.get_residue("A", 10, "").get_atom_by_atom_id("O");
 
 	auto sp1 = c.symmetry_copy(a.get_location(), "2_565"_symop);
@@ -442,10 +481,9 @@ TEST_CASE("symm_4wvp_1, *utf::tolerance(0.1f)")
 	REQUIRE_THAT(sp2.m_x, Catch::Matchers::WithinAbs(p.m_x, 0.5f));
 	REQUIRE_THAT(sp2.m_y, Catch::Matchers::WithinAbs(p.m_y, 0.5f));
 	REQUIRE_THAT(sp2.m_z, Catch::Matchers::WithinAbs(p.m_z, 0.5f));
-
 }
 
-TEST_CASE("symm_2bi3_1, *utf::tolerance(0.1f)")
+TEST_CASE("symm_2bi3_1")
 {
 	cif::file f(gTestDir / "2bi3.cif.gz");
 
@@ -455,18 +493,15 @@ TEST_CASE("symm_2bi3_1, *utf::tolerance(0.1f)")
 	cif::crystal c(db);
 
 	auto struct_conn = db["struct_conn"];
-	for (const auto &[
-			asym1, seqid1, authseqid1, atomid1, symm1,
-			asym2, seqid2, authseqid2, atomid2, symm2,
-			dist] : struct_conn.find<
-				std::string,int,std::string,std::string,std::string,
-				std::string,int,std::string,std::string,std::string,
-				float>(
-			cif::key("ptnr1_symmetry") != "1_555" or cif::key("ptnr2_symmetry") != "1_555",
-			"ptnr1_label_asym_id", "ptnr1_label_seq_id", "ptnr1_auth_seq_id", "ptnr1_label_atom_id", "ptnr1_symmetry", 
-			"ptnr2_label_asym_id", "ptnr2_label_seq_id", "ptnr2_auth_seq_id", "ptnr2_label_atom_id", "ptnr2_symmetry", 
-			"pdbx_dist_value"
-		))
+	for (const auto &[asym1, seqid1, authseqid1, atomid1, symm1,
+			 asym2, seqid2, authseqid2, atomid2, symm2,
+			 dist] : struct_conn.find<std::string, int, std::string, std::string, std::string,
+			 std::string, int, std::string, std::string, std::string,
+			 float>(
+			 cif::key("ptnr1_symmetry") != "1_555" or cif::key("ptnr2_symmetry") != "1_555",
+			 "ptnr1_label_asym_id", "ptnr1_label_seq_id", "ptnr1_auth_seq_id", "ptnr1_label_atom_id", "ptnr1_symmetry",
+			 "ptnr2_label_asym_id", "ptnr2_label_seq_id", "ptnr2_auth_seq_id", "ptnr2_label_atom_id", "ptnr2_symmetry",
+			 "pdbx_dist_value"))
 	{
 		auto &r1 = s.get_residue(asym1, seqid1, authseqid1);
 		auto &r2 = s.get_residue(asym2, seqid2, authseqid2);
@@ -492,7 +527,7 @@ TEST_CASE("symm_2bi3_1, *utf::tolerance(0.1f)")
 	}
 }
 
-TEST_CASE("symm_2bi3_1a, *utf::tolerance(0.1f)")
+TEST_CASE("symm_2bi3_1a")
 {
 	using namespace cif::literals;
 
@@ -504,23 +539,20 @@ TEST_CASE("symm_2bi3_1a, *utf::tolerance(0.1f)")
 	auto struct_conn = db["struct_conn"];
 	auto atom_site = db["atom_site"];
 
-	for (const auto &[
-			asym1, seqid1, authseqid1, atomid1, symm1,
-			asym2, seqid2, authseqid2, atomid2, symm2,
-			dist] : struct_conn.find<
-				std::string,std::optional<int>,std::string,std::string,std::string,
-				std::string,std::optional<int>,std::string,std::string,std::string,
-				float>(
-			cif::key("ptnr1_symmetry") != "1_555" or cif::key("ptnr2_symmetry") != "1_555",
-			"ptnr1_label_asym_id", "ptnr1_label_seq_id", "ptnr1_auth_seq_id", "ptnr1_label_atom_id", "ptnr1_symmetry", 
-			"ptnr2_label_asym_id", "ptnr2_label_seq_id", "ptnr2_auth_seq_id", "ptnr2_label_atom_id", "ptnr2_symmetry", 
-			"pdbx_dist_value"
-		))
+	for (const auto &[asym1, seqid1, authseqid1, atomid1, symm1,
+			 asym2, seqid2, authseqid2, atomid2, symm2,
+			 dist] : struct_conn.find<std::string, std::optional<int>, std::string, std::string, std::string,
+			 std::string, std::optional<int>, std::string, std::string, std::string,
+			 float>(
+			 cif::key("ptnr1_symmetry") != "1_555" or cif::key("ptnr2_symmetry") != "1_555",
+			 "ptnr1_label_asym_id", "ptnr1_label_seq_id", "ptnr1_auth_seq_id", "ptnr1_label_atom_id", "ptnr1_symmetry",
+			 "ptnr2_label_asym_id", "ptnr2_label_seq_id", "ptnr2_auth_seq_id", "ptnr2_label_atom_id", "ptnr2_symmetry",
+			 "pdbx_dist_value"))
 	{
-		cif::point p1 = atom_site.find1<float,float,float>(
+		cif::point p1 = atom_site.find1<float, float, float>(
 			"label_asym_id"_key == asym1 and "label_seq_id"_key == seqid1 and "auth_seq_id"_key == authseqid1 and "label_atom_id"_key == atomid1,
 			"cartn_x", "cartn_y", "cartn_z");
-		cif::point p2 = atom_site.find1<float,float,float>(
+		cif::point p2 = atom_site.find1<float, float, float>(
 			"label_asym_id"_key == asym2 and "label_seq_id"_key == seqid2 and "auth_seq_id"_key == authseqid2 and "label_atom_id"_key == atomid2,
 			"cartn_x", "cartn_y", "cartn_z");
 
@@ -540,7 +572,7 @@ TEST_CASE("symm_2bi3_1a, *utf::tolerance(0.1f)")
 	}
 }
 
-TEST_CASE("symm_3bwh_1, *utf::tolerance(0.1f)")
+TEST_CASE("symm_3bwh_1")
 {
 	cif::file f(gTestDir / "3bwh.cif.gz");
 
@@ -555,15 +587,15 @@ TEST_CASE("symm_3bwh_1, *utf::tolerance(0.1f)")
 		{
 			if (a1 == a2)
 				continue;
-			
-			const auto&[ d, p, so ] = c.closest_symmetry_copy(a1.get_location(), a2.get_location());
+
+			const auto &[d, p, so] = c.closest_symmetry_copy(a1.get_location(), a2.get_location());
 
 			REQUIRE_THAT(d, Catch::Matchers::WithinAbs(distance(a1.get_location(), p), 0.5f));
 		}
 	}
 }
 
-TEST_CASE("volume_3bwh_1, *utf::tolerance(0.1f)")
+TEST_CASE("volume_3bwh_1")
 {
 	cif::file f(gTestDir / "1juh.cif.gz");
 
@@ -573,4 +605,3 @@ TEST_CASE("volume_3bwh_1, *utf::tolerance(0.1f)")
 
 	REQUIRE_THAT(c.get_cell().get_volume(), Catch::Matchers::WithinRel(741009.625f, 0.01f));
 }
-

test/unit-v2-test.cpp

@@ -50,6 +50,23 @@ cif::file operator""_cf(const char *text, std::size_t length)
 
 // --------------------------------------------------------------------
 
+TEST_CASE("text_1")
+{
+	CHECK(cif::iequals("TEST", "test"));
+	CHECK(cif::iequals(std::string_view{"TEST"}, std::string_view{"test"}));
+
+	CHECK(cif::icompare("TEST", "test") == 0);
+	CHECK(cif::icompare(std::string_view{"TEST"}, std::string_view{"test"}) == 0);
+
+	CHECK(cif::icompare("TEST1", "test") > 0);
+	CHECK(cif::icompare(std::string_view{"TEST1"}, std::string_view{"test"}) > 0);
+
+	CHECK(cif::icompare("aap", "noot") < 0);
+	CHECK(cif::icompare(std::string_view{"aap"}, std::string_view{"noot"}) < 0);
+}
+
+// --------------------------------------------------------------------
+
 TEST_CASE("from_chars_1")
 {
 auto f = R"(data_TEST
@@ -70,8 +87,6 @@ _test.v
 	auto r2 = c.back();
 	REQUIRE(r2.get<double>("v") == 616.487);
 	REQUIRE(r2["v"].compare(616.487) == 0);
-
-
 }
 
 // --------------------------------------------------------------------
@@ -759,7 +774,6 @@ save__cat_2.desc
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -790,7 +804,7 @@ _cat_2.desc
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	SECTION("one")
 	{
@@ -912,7 +926,6 @@ save__cat_1.c
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -935,7 +948,7 @@ mies Mies
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	auto &cat1 = f.front()["cat_1"];
 
@@ -1074,7 +1087,6 @@ save__cat_2.desc
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -1108,7 +1120,7 @@ _cat_2.desc
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	auto &cat1 = f.front()["cat_1"];
 	auto &cat2 = f.front()["cat_2"];
@@ -1277,7 +1289,6 @@ save__cat_2.parent_id3
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -1321,7 +1332,7 @@ _cat_2.parent_id3
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	auto &cat1 = f.front()["cat_1"];
 	auto &cat2 = f.front()["cat_2"];
@@ -1498,7 +1509,6 @@ cat_2 3 cat_2:cat_1:3
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -1535,7 +1545,7 @@ _cat_2.parent_id3
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	auto &cat1 = f.front()["cat_1"];
 	auto &cat2 = f.front()["cat_2"];
@@ -1738,7 +1748,6 @@ cat_2 1 cat_2:cat_1:1
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -1775,7 +1784,7 @@ _cat_2.parent_id_2
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	// auto &cat1 = f.front()["cat_1"];
 	auto &cat2 = f.front()["cat_2"];
@@ -2134,7 +2143,6 @@ cat_2 1 '_cat_2.num'  '_cat_3.num'  cat_3
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -2176,7 +2184,7 @@ _cat_3.num
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	auto &cat1 = f.front()["cat_1"];
 	auto &cat2 = f.front()["cat_2"];
@@ -2419,7 +2427,6 @@ cat_2 1 '_cat_2.num'  '_cat_3.num'  cat_3
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -2461,7 +2468,7 @@ _cat_3.num
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	auto &cat1 = f.front()["cat_1"];
 	auto &cat2 = f.front()["cat_2"];
@@ -3016,7 +3023,6 @@ save__cat_1.name
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -3039,7 +3045,7 @@ _cat_1.name
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	REQUIRE(f.is_valid());
 
@@ -3211,7 +3217,6 @@ save__cat_1.name
 	auto &validator = cif::validator_factory::instance().construct_validator("test_dict.dic", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -3238,7 +3243,7 @@ _cat_1.name
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	REQUIRE(f.is_valid());
 
@@ -3318,7 +3323,6 @@ save__cat_1.id_2
 	auto validator = cif::parse_dictionary("test", is_dict);
 
 	cif::file f;
-	f.set_validator(&validator);
 
 	// --------------------------------------------------------------------
 
@@ -3343,7 +3347,7 @@ _cat_1.id_2
 	} data_buffer(const_cast<char *>(data), sizeof(data) - 1);
 
 	std::istream is_data(&data_buffer);
-	f.load(is_data);
+	f.load(is_data, validator);
 
 	auto &cat1 = f.front()["cat_1"];
 
@@ -3482,7 +3486,7 @@ TEST_CASE("compound_test_1")
 	cif::compound_factory::instance().push_dictionary(gTestDir / "REA_v2.cif");
 	auto compound = cif::compound_factory::instance().create("REA_v2");
 	REQUIRE(compound != nullptr);
-	REQUIRE(compound->id() == "REA_v2");
+	REQUIRE(cif::iequals(compound->id(), "REA_v2"));
 }
 
 // --------------------------------------------------------------------