Here is a patch with various important and minor patches to embed. These include - change some of the io errors to throw exceptions rather than return error codes as the error codes pretty much never checked in EMbed and don't propagate out. It also checks error codes more often for good measure) - write error messages to stderr instead of standard output - add a method to access a trilinearly interpolated density (so that the returned values are continuous) - initialize the data in density maps to 0 on construction - add a version of get_origin with takes an int (like the get-max) - add a few more error checks - tweak the endian hack in MRCReader since it was not getting the endianness right on my files.
Daniel Russel wrote: > Here is a patch with various important and minor patches to embed.
Sounds fine to me, but EMBED is not part of IMP, so there's little point in sending patches to the IMP mailing list. Send them to Keren instead.
Ben
keren will be the appropriate addressee.
anyway, some comments from my side: - thanks for propping up the error handling - for the endian: best provide a unit test and make sure your files (mrc?) are correct! for example, if your magic number indicates big endian, but the data are actually stored as little endian, your output will be messed up - but the reader works perfectly as it should do. so best take an example from the EMDep for the unit test. might well be there is a problem, but make sure it is not due to messy files. - in DensityHeader there is a confusing comment: ... float Objectpixelsize; //this is the actual pixelsize float Microscope; //Microscope + //! Use this to set the size of the voxel float Pixelsize; //Pixelsize - used for the microscope CCD camera ... the comment should probably apply to Objectpixelsize.
best
frido
On Jul 16, 2008, at 1:47 AM, Daniel Russel wrote:
> <patches>
--
Friedrich Foerster Max-Planck Institut fuer Biochemie Am Klopferspitz 18 D-82152 Martinsried
Tel: +49 89 8578 2651
foerster@biochem.mpg.de
www.tomotronic.org
> - in DensityHeader there is a confusing comment: > ... > float Objectpixelsize; //this is the actual pixelsize > float Microscope; //Microscope > + //! Use this to set the size of the voxel > float Pixelsize; //Pixelsize - used for the microscope CCD camera > ... > the comment should probably apply to Objectpixelsize. You all know better than me. I added that comment a while ago in response to something, but I don't remember why any more and haven't used that member since.
> for example, if your magic number indicates big > endian, but the data are actually stored as little endian, your output > will be messed up - but the reader works perfectly as it should do. Currently, as far as I can tell, there is no checking of any magic number in MRCReaderWriter. The only thing which controls the byte ordering is the hack having to do with looking for excessively large (and now excessively small) voxels.
> Sounds fine to me, but EMBED is not part of IMP, so there's little > point There are mostly on the list and it is archived :-) And it is an IMP dependency.
Daniel Russel wrote: >> for example, if your magic number indicates big >> endian, but the data are actually stored as little endian, your output >> will be messed up - but the reader works perfectly as it should do. > Currently, as far as I can tell, there is no checking of any magic > number in MRCReaderWriter. The only thing which controls the byte > ordering is the hack having to do with looking for excessively large > (and now excessively small) voxels.
The machine stamp ('magic number') is not reliably written into all MRC files, hence the workaround to detect byte-swapped files. MRC files written out, on the other hand, should have correct machine stamps.
Ben
Daniel Russel wrote: >> Sounds fine to me, but EMBED is not part of IMP, so there's little >> point > There are mostly on the list and it is archived :-) And it is an IMP > dependency.
No, there is an impEM module which uses EMBED. EMBED is certainly not required to use IMP.
Ben
On Jul 16, 2008, at 10:16 AM, Ben Webb wrote:
> Daniel Russel wrote: >>> for example, if your magic number indicates big >>> endian, but the data are actually stored as little endian, your >>> output >>> will be messed up - but the reader works perfectly as it should do. >> Currently, as far as I can tell, there is no checking of any magic >> number in MRCReaderWriter. The only thing which controls the byte >> ordering is the hack having to do with looking for excessively large >> (and now excessively small) voxels. > > The machine stamp ('magic number') is not reliably written into all > MRC > files, hence the workaround to detect byte-swapped files. MRC files > written out, on the other hand, should have correct machine stamps. Ick. Currently only the header is byteswapped if the machine stamp says the endianness is wrong. It seems like the data probably should be too. And then the heuristic applied.
>> There are mostly on the list and it is archived :-) And it is an IMP >> dependency. > > No, there is an impEM module which uses EMBED. EMBED is certainly not > required to use IMP. An optional dependency is still a dependency :-)
Daniel Russel wrote: > Ick. Currently only the header is byteswapped if the machine stamp > says the endianness is wrong. It seems like the data probably should > be too. And then the heuristic applied.
MRC files suck, so whatever you do will only work some of the time. You will also find MRC files with byte-swapped headers but not byte-swapped data, for example. There are also two different file types that use 'MRC' format (some of the header fields mean entirely different things) but of course there is no way to reliably tell the difference. The best we can do is to write out correct files, and do our best to figure out the inputs. If you're going to mess with the heuristics, Frido's suggestion is the best one - you add the suspect MRC input file to the testcases, so that further tweaks to the heuristics don't break existing files. I believe EMBED used my testcases for Modeller's MRC reader, but these can certainly be expanded.
Ben
On Jul 16, 2008, at 10:34 AM, Ben Webb wrote:
> Daniel Russel wrote: >> Ick. Currently only the header is byteswapped if the machine stamp >> says the endianness is wrong. It seems like the data probably should >> be too. And then the heuristic applied. > > MRC files suck, so whatever you do will only work some of the time. > You > will also find MRC files with byte-swapped headers but not byte- > swapped > data, for example. There are also two different file types that use > 'MRC' format (some of the header fields mean entirely different > things) > but of course there is no way to reliably tell the difference. The > best > we can do is to write out correct files, and do our best to figure out > the inputs. Sure. I was just questioning if flipping the bytes in the header entirely independently from the body (as is currently done) is really the right way to go about unmangling the files. I would have thought that the two would use the same ordering most of the time. But with biologists writing code, you never know :-)
hey Daniel,
We are slowly adding your patches to embed. Whoever is interested should ask Ben to be part of the embed mailing list.
Keren. On Jul 16, 2008, at 1:47 AM, Daniel Russel wrote:
> Here is a patch with various important and minor patches to embed. > These include > - change some of the io errors to throw exceptions rather than > return error codes as the error codes pretty much never checked in > EMbed and don't propagate out. It also checks error codes more often > for good measure) > - write error messages to stderr instead of standard output > - add a method to access a trilinearly interpolated density (so that > the returned values are continuous) > - initialize the data in density maps to 0 on construction > - add a version of get_origin with takes an int (like the get-max) > - add a few more error checks > - tweak the endian hack in MRCReader since it was not getting the > endianness right on my files. > > <patches> > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev
-
Ben Webb -
Daniel Russel -
Daniel Russel -
Friedrich Foerster -
Keren Lasker