From 61565336125a999790fb4c36219e9c46c5eb30cc Mon Sep 17 00:00:00 2001
From: Thomas White <taw@physics.org>
Date: Fri, 3 Mar 2017 17:37:10 +0100
Subject: Allow indexing system to store its own data, independently of
 indexing methods

Previously, the indexing system passed all the information on to the
indexing engines and then forgot about it.  That made it difficult to do
things like check the indexing solution after prediction refinement,
because the target unit cell was unavailable.  Now, the indexing system
itself can keep some information.  Of course, that information includes
the private pointers for the indexing engines themselves.

I took the opportunity to streamline things a little bit.  The caller
can now set up the indexing system in one step, without having to
separately parse the names of the indexing methods.  The caller no
longer has to keep track of a separate array of methods, instead just
one structure which contains everything.
---
 libcrystfel/src/index.c  | 358 ++++++++++++++++++++++++++++-------------------
 libcrystfel/src/index.h  |  26 ++--
 libcrystfel/src/stream.c |   8 +-
 3 files changed, 232 insertions(+), 160 deletions(-)

(limited to 'libcrystfel')

diff --git a/libcrystfel/src/index.c b/libcrystfel/src/index.c
index a2fdb861..89a98920 100644
--- a/libcrystfel/src/index.c
+++ b/libcrystfel/src/index.c
@@ -58,6 +58,17 @@
 #include "predict-refine.h"
 
 
+struct _indexingprivate
+{
+	UnitCell *target_cell;
+	float tolerance[4];
+
+	int n_methods;
+	IndexingMethod *methods;
+	void **engine_private;
+};
+
+
 static int debug_index(struct image *image)
 {
 	Crystal *cr = crystal_new();
@@ -71,74 +82,105 @@ static int debug_index(struct image *image)
 }
 
 
-IndexingPrivate **prepare_indexing(IndexingMethod *indm, UnitCell *cell,
-                                   struct detector *det, float *ltl,
-                                   const char *options)
+static void *prepare_method(IndexingMethod *m, UnitCell *cell,
+                            struct detector *det, float *ltl,
+                            const char *options)
 {
-	int n;
-	int nm = 0;
-	IndexingPrivate **iprivs;
+	char *str;
+	IndexingMethod in = *m;
+	void *priv = NULL;
 
-	while ( indm[nm] != INDEXING_NONE ) nm++;
-	iprivs = malloc((nm+1) * sizeof(IndexingPrivate *));
+	switch ( *m & INDEXING_METHOD_MASK ) {
 
-	for ( n=0; n<nm; n++ ) {
+		case INDEXING_DIRAX :
+		priv = dirax_prepare(m, cell, det, ltl);
+		break;
 
-		int i;
-		IndexingMethod in;
-		char *str;
+		case INDEXING_ASDF :
+		priv = asdf_prepare(m, cell, det, ltl);
+		break;
 
-		in = indm[n];
+		case INDEXING_MOSFLM :
+		priv = mosflm_prepare(m, cell, det, ltl);
+		break;
 
-		switch ( indm[n] & INDEXING_METHOD_MASK ) {
+		case INDEXING_XDS :
+		priv = xds_prepare(m, cell, det, ltl);
+		break;
 
-			case INDEXING_DIRAX :
-			iprivs[n] = dirax_prepare(&indm[n], cell, det, ltl);
-			break;
+		case INDEXING_DEBUG :
+		priv = (IndexingPrivate *)strdup("Hello!");
+		break;
 
-			case INDEXING_ASDF :
-			iprivs[n] = asdf_prepare(&indm[n], cell, det, ltl);
-			break;
+		case INDEXING_FELIX :
+		priv = felix_prepare(m, cell, det, ltl, options);
+		break;
 
-			case INDEXING_MOSFLM :
-			iprivs[n] = mosflm_prepare(&indm[n], cell, det, ltl);
-			break;
+		default :
+		ERROR("Don't know how to prepare indexing method %i\n", *m);
+		break;
 
-			case INDEXING_XDS :
-			iprivs[n] = xds_prepare(&indm[n], cell, det, ltl);
-			break;
+	}
 
-			case INDEXING_DEBUG :
-			iprivs[n] = (IndexingPrivate *)strdup("Hello!");
-			break;
+	str = indexer_str(*m);
 
-			case INDEXING_FELIX :
-			iprivs[n] = felix_prepare(&indm[n], cell, det, ltl,
-			                          options);
-			break;
+	if ( priv == NULL ) {
+		ERROR("Failed to prepare indexing method %s\n", str);
+		free(str);
+		return NULL;
+	}
 
-			default :
-			ERROR("Don't know how to prepare indexing method %i\n",
-			      indm[n]);
-			break;
+	STATUS("Prepared indexing method %s\n", str);
+	free(str);
 
-		}
+	if ( in != *m ) {
+		ERROR("Note: flags were altered to take into account "
+		      "the information provided and/or the limitations "
+		      "of the indexing method.\nPlease check the "
+		      "methods listed above carefully.\n");
+	}
 
-		if ( iprivs[n] == NULL ) return NULL;
+	return priv;
+}
 
-		str = indexer_str(indm[n]);
-		STATUS("Prepared indexing method %i: %s\n", n, str);
-		free(str);
 
-		if ( in != indm[n] ) {
-			ERROR("Note: flags were altered to take into account "
-			      "the information provided and/or the limitations "
-			      "of the indexing method.\nPlease check the "
-			      "methods listed above carefully.\n");
-		}
+IndexingPrivate *setup_indexing(const char *method_list, UnitCell *cell,
+                                struct detector *det, float *ltl,
+                                int no_refine, const char *options)
+{
+	int i, n;
+	char **method_strings;
+	IndexingPrivate *ipriv;
+
+	/* Parse indexing methods */
+	n = assplode(method_list, ",", &method_strings, ASSPLODE_NONE);
+
+	IndexingMethod *methods = malloc(n * sizeof(IndexingMethod));
+	if ( methods == NULL ) {
+		ERROR("Failed to allocate indexing method list\n");
+		return NULL;
+	}
+
+	for ( i=0; i<n; i++ ) {
+		methods[i] = get_indm_from_string(method_strings[i]);
+	}
+
+	ipriv = malloc(sizeof(struct _indexingprivate));
+	if ( ipriv == NULL ) {
+		ERROR("Failed to allocate indexing data\n");
+		return NULL;
+	}
+
+	ipriv->engine_private = malloc((n+1) * sizeof(void *));
+
+	for ( i=0; i<n; i++ ) {
+
+		int j;
 
-		for ( i=0; i<n; i++ ) {
-			if ( indm[i] == indm[n] ) {
+		ipriv->engine_private[i] = prepare_method(&methods[i], cell,
+		                                          det, ltl, options);
+		for ( j=0; j<i; j++ ) {
+			if ( methods[i] == methods[j] ) {
 				ERROR("Duplicate indexing method.\n");
 				ERROR("Have you specified some flags which "
 				      "aren't accepted by one of your "
@@ -147,55 +189,62 @@ IndexingPrivate **prepare_indexing(IndexingMethod *indm, UnitCell *cell,
 			}
 		}
 
+	}
+
+	ipriv->methods = methods;
+	ipriv->n_methods = n;
 
+	if ( cell != NULL ) {
+		ipriv->target_cell = cell_new_from_cell(cell);
+	} else {
+		ipriv->target_cell = NULL;
 	}
-	iprivs[n] = NULL;
+	for ( i=0; i<4; i++ ) ipriv->tolerance[i] = ltl[i];
 
-	return iprivs;
+	return ipriv;
 }
 
 
-void cleanup_indexing(IndexingMethod *indms, IndexingPrivate **privs)
+void cleanup_indexing(IndexingPrivate *ipriv)
 {
-	int n = 0;
+	int n;
 
-	if ( indms == NULL ) return;  /* Nothing to do */
-	if ( privs == NULL ) return;  /* Nothing to do */
+	if ( ipriv == NULL ) return;  /* Nothing to do */
 
-	while ( indms[n] != INDEXING_NONE ) {
+	for ( n=0; n<ipriv->n_methods; n++ ) {
 
-		switch ( indms[n] & INDEXING_METHOD_MASK ) {
+		switch ( ipriv->methods[n] & INDEXING_METHOD_MASK ) {
 
 			case INDEXING_NONE :
 			break;
 
 			case INDEXING_DIRAX :
-			dirax_cleanup(privs[n]);
+			dirax_cleanup(ipriv->engine_private[n]);
 			break;
 
 			case INDEXING_ASDF :
-			asdf_cleanup(privs[n]);
+			asdf_cleanup(ipriv->engine_private[n]);
 			break;
 
 			case INDEXING_MOSFLM :
-			mosflm_cleanup(privs[n]);
+			mosflm_cleanup(ipriv->engine_private[n]);
 			break;
 
 			case INDEXING_XDS :
-			xds_cleanup(privs[n]);
+			xds_cleanup(ipriv->engine_private[n]);
 			break;
 
 			case INDEXING_FELIX :
-			felix_cleanup(privs[n]);
+			felix_cleanup(ipriv->engine_private[n]);
 			break;
 
 			case INDEXING_DEBUG :
-			free(privs[n]);
+			free(ipriv->engine_private[n]);
 			break;
 
 			default :
 			ERROR("Don't know how to clean up indexing method %i\n",
-			      indms[n]);
+			      ipriv->methods[n]);
 			break;
 
 		}
@@ -204,8 +253,10 @@ void cleanup_indexing(IndexingMethod *indms, IndexingPrivate **privs)
 
 	}
 
-	free(indms);
-	free(privs);
+	free(ipriv->methods);
+	free(ipriv->engine_private);
+	cell_free(ipriv->target_cell);
+	free(ipriv);
 }
 
 
@@ -234,7 +285,7 @@ void map_all_peaks(struct image *image)
 
 /* Return non-zero for "success" */
 static int try_indexer(struct image *image, IndexingMethod indm,
-                       IndexingPrivate *ipriv)
+                       IndexingPrivate *ipriv, void *mpriv)
 {
 	int i, r;
 	int n_bad = 0;
@@ -245,19 +296,19 @@ static int try_indexer(struct image *image, IndexingMethod indm,
 		return 0;
 
 		case INDEXING_DIRAX :
-		r = run_dirax(image, ipriv);
+		r = run_dirax(image, mpriv);
 		break;
 
 		case INDEXING_ASDF :
-		r = run_asdf(image, ipriv);
+		r = run_asdf(image, mpriv);
 		break;
 
 		case INDEXING_MOSFLM :
-		r = run_mosflm(image, ipriv);
+		r = run_mosflm(image, mpriv);
 		break;
 
 		case INDEXING_XDS :
-		r = run_xds(image, ipriv);
+		r = run_xds(image, mpriv);
 		break;
 
 		case INDEXING_DEBUG :
@@ -265,7 +316,7 @@ static int try_indexer(struct image *image, IndexingMethod indm,
 		break;
 
 		case INDEXING_FELIX :
-		r = felix_index(image, ipriv);
+		r = felix_index(image, mpriv);
 		break;
 
 		default :
@@ -421,20 +472,18 @@ static int finished_retry(IndexingMethod indm, int r, struct image *image)
 	}
 }
 
-void index_pattern(struct image *image,
-                   IndexingMethod *indms, IndexingPrivate **iprivs)
+void index_pattern(struct image *image, IndexingPrivate *ipriv)
 {
-	index_pattern_2(image, indms, iprivs, NULL);
+	index_pattern_2(image, ipriv, NULL);
 }
 
 
-void index_pattern_2(struct image *image, IndexingMethod *indms,
-                     IndexingPrivate **iprivs, int *ping)
+void index_pattern_2(struct image *image, IndexingPrivate *ipriv, int *ping)
 {
 	int n = 0;
 	ImageFeatureList *orig;
 
-	if ( indms == NULL ) return;
+	if ( ipriv == NULL ) return;
 
 	map_all_peaks(image);
 	image->crystals = NULL;
@@ -442,7 +491,7 @@ void index_pattern_2(struct image *image, IndexingMethod *indms,
 
 	orig = image->features;
 
-	while ( indms[n] != INDEXING_NONE ) {
+	for ( n=0; n<ipriv->n_methods; n++ ) {
 
 		int done = 0;
 		int r;
@@ -453,10 +502,11 @@ void index_pattern_2(struct image *image, IndexingMethod *indms,
 
 		do {
 
-			r = try_indexer(image, indms[n], iprivs[n]);
+			r = try_indexer(image, ipriv->methods[n],
+			                ipriv, ipriv->engine_private[n]);
 			success += r;
 			ntry++;
-			done = finished_retry(indms[n], r, image);
+			done = finished_retry(ipriv->methods[n], r, image);
 			if ( ntry > 5 ) done = 1;
 			if ( ping != NULL ) (*ping)++;
 
@@ -468,11 +518,14 @@ void index_pattern_2(struct image *image, IndexingMethod *indms,
 		 * crystals with a different indexing method) */
 		if ( success ) break;
 
-		n++;
+	}
 
+	if ( n < ipriv->n_methods ) {
+		image->indexed_by = ipriv->methods[n];
+	} else {
+		image->indexed_by = INDEXING_NONE;
 	}
 
-	image->indexed_by = indms[n];
 	image->features = orig;
 }
 
@@ -639,97 +692,118 @@ char *indexer_str(IndexingMethod indm)
 }
 
 
-IndexingMethod *build_indexer_list(const char *str)
+static IndexingMethod warn_method(const char *str)
+{
+	ERROR("Indexing method must contain exactly one engine name: '%s'\n",
+	      str);
+	return INDEXING_ERROR;
+}
+
+
+IndexingMethod get_indm_from_string(const char *str)
 {
 	int n, i;
-	char **methods;
-	IndexingMethod *list;
-	int nmeth = 0;
+	char **bits;
+	IndexingMethod method = INDEXING_NONE;
+	int have_method = 0;
 
-	n = assplode(str, ",-", &methods, ASSPLODE_NONE);
-	list = malloc((n+1)*sizeof(IndexingMethod));
+	n = assplode(str, "-", &bits, ASSPLODE_NONE);
 
-	nmeth = -1;  /* So that the first method is #0 */
 	for ( i=0; i<n; i++ ) {
 
-		if ( strcmp(methods[i], "dirax") == 0) {
-			list[++nmeth] = INDEXING_DEFAULTS_DIRAX;
+		if ( strcmp(bits[i], "dirax") == 0) {
+			if ( have_method ) return warn_method(str);
+			method = INDEXING_DEFAULTS_DIRAX;
+			have_method = 1;
 
-		} else if ( strcmp(methods[i], "asdf") == 0) {
-			list[++nmeth] = INDEXING_DEFAULTS_ASDF;
+		} else if ( strcmp(bits[i], "asdf") == 0) {
+			if ( have_method ) return warn_method(str);
+			method = INDEXING_DEFAULTS_ASDF;
+			have_method = 1;
 
-		} else if ( strcmp(methods[i], "mosflm") == 0) {
-			list[++nmeth] = INDEXING_DEFAULTS_MOSFLM;
+		} else if ( strcmp(bits[i], "mosflm") == 0) {
+			if ( have_method ) return warn_method(str);
+			method = INDEXING_DEFAULTS_MOSFLM;
+			have_method = 1;
 
-		} else if ( strcmp(methods[i], "xds") == 0) {
-			list[++nmeth] = INDEXING_DEFAULTS_XDS;
+		} else if ( strcmp(bits[i], "xds") == 0) {
+			if ( have_method ) return warn_method(str);
+			method = INDEXING_DEFAULTS_XDS;
+			have_method = 1;
 
-		} else if ( strcmp(methods[i], "felix") == 0) {
-			list[++nmeth] = INDEXING_DEFAULTS_FELIX;
+		} else if ( strcmp(bits[i], "felix") == 0) {
+			if ( have_method ) return warn_method(str);
+			method = INDEXING_DEFAULTS_FELIX;
+			have_method = 1;
 
-		} else if ( strcmp(methods[i], "none") == 0) {
-			list[++nmeth] = INDEXING_NONE;
+		} else if ( strcmp(bits[i], "none") == 0) {
+			if ( have_method ) return warn_method(str);
+			method = INDEXING_NONE;
+			have_method = 1;
 
-		} else if ( strcmp(methods[i], "simulation") == 0) {
-			list[++nmeth] = INDEXING_SIMULATION;
-			return list;
+		} else if ( strcmp(bits[i], "simulation") == 0) {
+			if ( have_method ) return warn_method(str);
+			method = INDEXING_SIMULATION;
+			return method;
 
-		} else if ( strcmp(methods[i], "debug") == 0) {
-			list[++nmeth] = INDEXING_DEBUG;
-			return list;
+		} else if ( strcmp(bits[i], "debug") == 0) {
+			if ( have_method ) return warn_method(str);
+			method = INDEXING_DEBUG;
+			return method;
 
-		} else if ( strcmp(methods[i], "raw") == 0) {
-			list[nmeth] = set_raw(list[nmeth]);
+		} else if ( strcmp(bits[i], "raw") == 0) {
+			method = set_raw(method);
 
-		} else if ( strcmp(methods[i], "bad") == 0) {
-			list[nmeth] = set_bad(list[nmeth]);
+		} else if ( strcmp(bits[i], "bad") == 0) {
+			method = set_bad(method);
 
-		} else if ( strcmp(methods[i], "comb") == 0) {
-			list[nmeth] = set_comb(list[nmeth]);  /* Default */
+		} else if ( strcmp(bits[i], "comb") == 0) {
+			method = set_comb(method);  /* Default */
 
-		} else if ( strcmp(methods[i], "axes") == 0) {
-			list[nmeth] = set_axes(list[nmeth]);
+		} else if ( strcmp(bits[i], "axes") == 0) {
+			method = set_axes(method);
 
-		} else if ( strcmp(methods[i], "latt") == 0) {
-			list[nmeth] = set_lattice(list[nmeth]);
+		} else if ( strcmp(bits[i], "latt") == 0) {
+			method = set_lattice(method);
 
-		} else if ( strcmp(methods[i], "nolatt") == 0) {
-			list[nmeth] = set_nolattice(list[nmeth]);
+		} else if ( strcmp(bits[i], "nolatt") == 0) {
+			method = set_nolattice(method);
 
-		} else if ( strcmp(methods[i], "cell") == 0) {
-			list[nmeth] = set_cellparams(list[nmeth]);
+		} else if ( strcmp(bits[i], "cell") == 0) {
+			method = set_cellparams(method);
 
-		} else if ( strcmp(methods[i], "nocell") == 0) {
-			list[nmeth] = set_nocellparams(list[nmeth]);
+		} else if ( strcmp(bits[i], "nocell") == 0) {
+			method = set_nocellparams(method);
 
-		} else if ( strcmp(methods[i], "retry") == 0) {
-			list[nmeth] |= INDEXING_RETRY;
+		} else if ( strcmp(bits[i], "retry") == 0) {
+			method |= INDEXING_RETRY;
 
-		} else if ( strcmp(methods[i], "noretry") == 0) {
-			list[nmeth] &= ~INDEXING_RETRY;
+		} else if ( strcmp(bits[i], "noretry") == 0) {
+			method &= ~INDEXING_RETRY;
 
-		} else if ( strcmp(methods[i], "multi") == 0) {
-			list[nmeth] |= INDEXING_MULTI;
+		} else if ( strcmp(bits[i], "multi") == 0) {
+			method |= INDEXING_MULTI;
 
-		} else if ( strcmp(methods[i], "nomulti") == 0) {
-			list[nmeth] &= ~INDEXING_MULTI;
+		} else if ( strcmp(bits[i], "nomulti") == 0) {
+			method &= ~INDEXING_MULTI;
 
-		} else if ( strcmp(methods[i], "refine") == 0) {
-			list[nmeth] |= INDEXING_REFINE;
+		} else if ( strcmp(bits[i], "refine") == 0) {
+			method |= INDEXING_REFINE;
 
-		} else if ( strcmp(methods[i], "norefine") == 0) {
-			list[nmeth] &= ~INDEXING_REFINE;
+		} else if ( strcmp(bits[i], "norefine") == 0) {
+			method &= ~INDEXING_REFINE;
 
 		} else {
 			ERROR("Bad list of indexing methods: '%s'\n", str);
-			return NULL;
+			return INDEXING_ERROR;
 		}
 
-		free(methods[i]);
+		free(bits[i]);
 
 	}
-	free(methods);
-	list[++nmeth] = INDEXING_NONE;
+	free(bits);
+
+	if ( !have_method ) return warn_method(str);
 
-	return list;
+	return method;
 }
diff --git a/libcrystfel/src/index.h b/libcrystfel/src/index.h
index e5f7764a..8ce553ae 100644
--- a/libcrystfel/src/index.h
+++ b/libcrystfel/src/index.h
@@ -84,7 +84,7 @@
  * @INDEXING_USE_LATTICE_TYPE: Use lattice type and centering information to
  *   guide the indexing process.
  * @INDEXING_USE_CELL_PARAMETERS: Use the unit cell parameters to guide the
- *   indexingprocess.
+ *   indexing process.
  * @INDEXING_RETRY: If the indexer doesn't succeed, delete the weakest peaks
  *   and try again.
  * @INDEXING_MULTI: If the indexer succeeds, delete the peaks explained by the
@@ -108,6 +108,8 @@ typedef enum {
 	INDEXING_DEBUG = 7,
 	INDEXING_ASDF = 8,
 
+	INDEXING_ERROR = 255,  /* Unrecognised indexing engine */
+
 	/* Bits at the top of the IndexingMethod are flags which modify the
 	 * behaviour of the indexer. */
 	INDEXING_CHECK_CELL_COMBINATIONS = 256,
@@ -136,28 +138,28 @@ extern "C" {
  * IndexingPrivate:
  *
  * This is an opaque data structure containing information needed by the
- * indexing method.
+ * indexing system.
  **/
-typedef void *IndexingPrivate;
+typedef struct _indexingprivate IndexingPrivate;
 
-extern IndexingMethod *build_indexer_list(const char *str);
+/* Convert indexing methods to/from text */
 extern char *indexer_str(IndexingMethod indm);
+extern IndexingMethod get_indm_from_string(const char *method);
 
 #include "detector.h"
 #include "cell.h"
 #include "image.h"
 
-extern IndexingPrivate **prepare_indexing(IndexingMethod *indm, UnitCell *cell,
-                                          struct detector *det, float *ltl,
-                                          const char *options);
+extern IndexingPrivate *setup_indexing(const char *methods, UnitCell *cell,
+                                       struct detector *det, float *ltl,
+                                       int no_refine, const char *options);
 
-extern void index_pattern(struct image *image,
-                          IndexingMethod *indms, IndexingPrivate **iprivs);
+extern void index_pattern(struct image *image, IndexingPrivate *ipriv);
 
-extern void index_pattern_2(struct image *image, IndexingMethod *indms,
-                            IndexingPrivate **iprivs, int *ping);
+extern void index_pattern_2(struct image *image, IndexingPrivate *ipriv,
+                            int *ping);
 
-extern void cleanup_indexing(IndexingMethod *indms, IndexingPrivate **privs);
+extern void cleanup_indexing(IndexingPrivate *ipriv);
 
 #ifdef __cplusplus
 }
diff --git a/libcrystfel/src/stream.c b/libcrystfel/src/stream.c
index e858172e..17da74b2 100644
--- a/libcrystfel/src/stream.c
+++ b/libcrystfel/src/stream.c
@@ -1196,13 +1196,9 @@ int read_chunk_2(Stream *st, struct image *image,  StreamReadFlags srf)
 		}
 
 		if ( strncmp(line, "indexed_by = ", 13) == 0 ) {
-			IndexingMethod *list;
-			list = build_indexer_list(line+13);
-			if ( list == NULL ) {
+			image->indexed_by = get_indm_from_string(line+13);
+			if ( image->indexed_by == INDEXING_ERROR ) {
 				ERROR("Failed to read indexer list\n");
-			} else {
-				image->indexed_by = list[0];
-				free(list);
 			}
 		}
 
-- 
cgit v1.2.3