3dem / relion

Image-processing software for cryo-electron microscopy
https://relion.readthedocs.io/en/latest/
GNU General Public License v2.0
446 stars 200 forks source link

Writing a STAR file with unknown labels sometimes fails #641

Closed biochem-fan closed 3 years ago

biochem-fan commented 4 years ago

Reported by Andres Gonzales.

Somehow unknownLabelNames is empty...

biochem-fan commented 4 years ago

This is caused byaddMissingLabels() failing to set unknowLabelNames. Also have to check if unknownLabelPosition2Offset is valid after deleting some labels.

biochem-fan commented 3 years ago

label2offset[l] < 0: This check is not sufficient with more than one unknownLabels.

biochem-fan commented 3 years ago

activeLabel mechanism is not good; we cannot distinguish different unknownLabels.

biochem-fan commented 3 years ago

label2Offset is the culprit. We might have several EMDL_UNKNOWN_LABELs.

biochem-fan commented 3 years ago

We need a map<string, int> for unknown labels. Oops, this is extremely messy. Perhaps we should use (int)EMDL_UNKNOWN_LABEL + iinstead? But label2offset needs a fixed size...

biochem-fan commented 3 years ago

We should not erase items from activeLabels but instead set to -1 or something. Let's call this labels. Then unknownLabelPosition2Offset becomes easier to maintain, since labelPos = physical pos in a table, constant even after deactivateLabel.

When we loop over labels (e.g. write or compare):

OR

Keep activeLabels as is, but change unknownLabelPosition2Offset to vector and erase the same element when activeLabels are updated. Then unknownLabelNames[unknownLabelPosition2Offset[i]] will give the column name.

biochem-fan commented 3 years ago

Need to test:

biochem-fan commented 3 years ago

desiredLabels mechanism can be removed? Currently it is broken.

biochem-fan commented 3 years ago

containsLabel and labelExists seem redundant.

biochem-fan commented 3 years ago

Working via the "Keep activeLabels as is, but change unknownLabelPosition2Offset to vector and erase the same element when activeLabels are updated" route.

deactivateLabel is OK now, combine needs more work.

biochem-fan commented 3 years ago

Fixed in f295c96.