leginon-org / leginon-redmine-archive

1 stars 0 forks source link

Image flipping for IMAGIC format in apImagicFile.py #4382

Open leginonbot opened 8 months ago

leginonbot commented 8 months ago

Author Name: Neil Voss (@vosslab) Original Redmine Issue: 4382, https://emg.nysbc.org/redmine/issues/4382 Original Date: 2016-08-14 Original Assignee: Anchi Cheng


The current code for apImagicFile.py when writing a set of particles to a stack and then reading them back in the data is flipped. I know we talked about this in Bug #1906, but it is back. Do you we want to flip the data or just leave it alone?

I think it is crucial to be able to write and read back without the data changing. I do not care if we flip or not, but needs to be consistent. I vote we just stop flipping.

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Gabriel Lander (@gclander) Original Date: 2016-08-15T05:16:23Z


I think this is only really important if we're using the stack for RCT, but I suppose there must be other places where this becomes important, or else why would we be flipping in the first place? I propose we stop flipping and see if anyone reports any bugs downstream.

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Scott Stagg (Scott Stagg) Original Date: 2016-08-15T13:09:39Z


I don't care whether we flip or not as long as ALL tools that rely on handedness are consistent with any changes. We still use RCT, and who knows, one day we may all have to use small angle tilts. My feeling is that we should always be consistent with the microscope. We should be consistent with the reported microscope tilt angle and viewing screen.

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Anchi Cheng (@anchi2c) Original Date: 2016-08-15T13:27:47Z


I am against changes (i.e., remove flip) unless Neil is willing to go through every tool and change them accordingly and write dbschema update as I did painfully for ctf estimation. It is not right to change and wait for users to report that it is broken. You will frustrate the users and make them walk away.

Does anyone know how EMAN2 handles mrc vs Imagic ? Does it still flip the data when converting to make them display in the same way ?

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Bridget Carragher (@bcarr-czi) Original Date: 2016-08-15T13:51:03Z


I agree with Anchi. If it ain't broke don't fix it!

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Anchi Cheng (@anchi2c) Original Date: 2016-08-15T13:55:07Z


It is not it ain't broken. I am just against letting user finding out that our change breaks what they do knowingly.

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Neil Voss (@vosslab) Original Date: 2016-08-15T13:56:46Z


I appreciate Anchi chiming in on this, because she is the most knowledgeable on the flipping subject. The reason I brought it up again is that it currently is broken.

import numpy
from appionlib import apImagicFile

a = numpy.random.random((4, 128, 128))
apImagicFile.writeImagic(a, filename) #flip and write images to file
partdata = []
for partnum in numpart:
    a = apImagicFile.readSingleParticleFromStack(filename, partnum=partnum)
    partdata.append(a)
b = numpy.array(partdata) # this data is still flipped.
#PROBLEM: a != b

I do not know how long this has been the case but it worries me.

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Scott Stagg (Scott Stagg) Original Date: 2016-08-15T14:01:36Z


This is all boiling down to the origin problem. Different packages use either the top left or bottom left as the origin. As we all know, this can result in a hand flip. It's also sort of an unsolvable problem because no matter what, you will always have disagreement between packages with different origins. The only way to really solve it is to have a db entry where whenever you write an image with associated metadata, you mark whether the metadata refer to a top left or a bottom left origin.

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Neil Voss (@vosslab) Original Date: 2016-08-15T14:04:52Z


As Scott says that reason I put the flip in originally was because proc2d does the flip. Proc2d did the flip because IMAGIC and MRC we being displayed differently in their native programs because of the image origin.

Question: does EMAN2 still do the flip between other stacks and IMAGIC?

leginonbot commented 8 months ago

Original Redmine Comment Author Name: Neil Voss (@vosslab) Original Date: 2016-08-15T19:20:11Z


I reverted removing the flips and I added flips to functions that were missing flips, but there is a lot of duplicated read/write code in apImagicFile.py and many of them were missing flips.

So, I would be hesitant to trust any IMAGIC files to have the correct mirror. Even now some functions may not be flipping or unflipping properly.

def readImagic(filename, first=1, last=None, msg=True):
def readImagicHeader(headerfilename):
def readIndexFromHeader(headerfilename, indexnum, numparts=100):
def readImagicData(datafilename, headerdict, firstpart=1, numpart=1):
def writeImagic(array, filename, msg=True):
def writeVarianceImage(imagicfile, varmrcfile):
def readSingleParticleFromStack(filename, partnum=1, boxsize=None, msg=True):
def appendParticleToStackFile(partarray, mergestackfile, msg=True):
def appendParticleListToStackFile(partlist, mergestackfile, msg=True):
def appendStackFileToStackFile(stackfile, mergestackfile, msg=True):
def mergeStacks(stacklist, mergestack, msg=True):
def readParticleListFromStack(filename, partlist, boxsize=None, msg=True):
def wrtieOddParticles(self, stackfile, outfile=None):
def wrtieEvenParticles(self, stackfile, outfile=None):
leginonbot commented 8 months ago

Original Redmine Comment Author Name: Gabriel Lander (@gclander) Original Date: 2016-08-15T20:14:00Z


I'm fairly certain that we added the flip originally because imagic and proc2d had different origins. Now that everything is being handled by Neil's python, everything should be consistent, and the flip shouldn't be necessary. But I suppose we need some kind of test to be sure.