Questionable detection of 8-bit-packed mode from MRC headers

[jcbollinger]

Having recently engaged in the exercise of building the EMAN2 2.3 release from source, I had the opportunity to analyze compiler warnings to look for any that indicated genuine problems. There was a lot of noise, but a little signal, all of which appears still to apply to the current head. I'll be splitting that into four separate issues, of which this is the first.

---

libEM/mrcio.cpp:206-207 is this:

		is_8_bit_packed = (mrch.mode == MRC_UHEX       ||
							   (mrch.imod_flags & 16) == 1  ||  double_nx);

But the condition (mrch.imod_flags & 16) == 1 never evaluates to true, because anything & 16 always evaluates to either 0 or 16. From code analysis alone, I suspect that (mrch.imod_flags & 16) != 0 would be consistent with the intention, but I am not sufficiently familiar with the details of the MRC format to confirm that.

[sludtke42]

Hi John, thanks for taking the time to work on some of these. Periodically when someone has free time, people do browse through the numerous warnings and fix things, but as you have undoubtedly noticed that academic code developed over 15 years with a couple of dozen, not always well-trained, programmers will tend to have its share of little issues :^) Anyway, we will get the correct person to look at each of these. I would suggest that if you're taking the time to do this, you might want to work on master with the Python3 prerelease rather than focusing on a year-old release version. I doubt the two things you've posted have changed, with, but we have moved pretty far past 2.3 at this point :^)

…

-------------------------------------------------------------------------------------- Steven Ludtke, Ph.D. <sludtke@bcm.edu <mailto:sludtke@bcm.edu>> Baylor College of Medicine Charles C. Bell Jr., Professor of Structural Biology Dept. of Biochemistry and Molecular Biology (www.bcm.edu/biochem <http://www.bcm.edu/biochem>) Academic Director, CryoEM Core (cryoem.bcm.edu <http://cryoem.bcm.edu/>) Co-Director CIBR Center (www.bcm.edu/research/cibr <http://www.bcm.edu/research/cibr>)

On Apr 10, 2020, at 10:43 AM, John Bollinger ***@***.***> wrote: Having recently engaged in the exercise of building the EMAN2 2.3 release from source, I had the opportunity to analyze compiler warnings to look for any that indicated genuine problems. There was a lot of noise, but a little signal, all of which appears still to apply to the current head. I'll be splitting that into four separate issues, of which this is the first. libEM/mrcio.cpp:206-207 is this: is_8_bit_packed = (mrch.mode == MRC_UHEX || (mrch.imod_flags & 16) == 1 || double_nx); But the condition (mrch.imod_flags & 16) == 1 never evaluates to true, because anything & 16 always evaluates to either 0 or 16. From code analysis alone, I suspect that (mrch.imod_flags & 16) != 0 would be consistent with the intention, but I am not sufficiently familiar with the details of the MRC format to confirm that. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#458>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPKF2UHSA64MSRFTSPF7BLRL45BDANCNFSM4MFQZ32A>.

[sludtke42]

Resolved on master (not pushed yet)

[jcbollinger]

Hi Steve, That was fast! I assure you that although I'm working with warnings generated while compiling the latest release version, I am checking the current Git head to verify that the problems appear to still be present there.

[sludtke42]

Ok, no problem :^)

…

-------------------------------------------------------------------------------------- Steven Ludtke, Ph.D. <sludtke@bcm.edu <mailto:sludtke@bcm.edu>> Baylor College of Medicine Charles C. Bell Jr., Professor of Structural Biology Dept. of Biochemistry and Molecular Biology (www.bcm.edu/biochem <http://www.bcm.edu/biochem>) Academic Director, CryoEM Core (cryoem.bcm.edu <http://cryoem.bcm.edu/>) Co-Director CIBR Center (www.bcm.edu/research/cibr <http://www.bcm.edu/research/cibr>)

On Apr 10, 2020, at 12:01 PM, John Bollinger ***@***.***> wrote: Hi Steve, That was fast! I assure you that although I'm working with warnings generated while compiling the latest release version, I am checking the current Git head to verify that the problems appear to still be present there. Cheers, John ________________________________ From: Steve Ludtke ***@***.*** ***@***.***>> Sent: Friday, April 10, 2020 11:18 AM To: cryoem/eman2 ***@***.*** ***@***.***>> Cc: Bollinger, John C ***@***.*** ***@***.***>>; Author ***@***.*** ***@***.***>> Subject: Re: [cryoem/eman2] Questionable detection of 8-bit-packed mode from MRC headers (#458) Caution: External Sender Hi John, thanks for taking the time to work on some of these. Periodically when someone has free time, people do browse through the numerous warnings and fix things, but as you have undoubtedly noticed that academic code developed over 15 years with a couple of dozen, not always well-trained, programmers will tend to have its share of little issues :^) Anyway, we will get the correct person to look at each of these. I would suggest that if you're taking the time to do this, you might want to work on master with the Python3 prerelease rather than focusing on a year-old release version. I doubt the two things you've posted have changed, with, but we have moved pretty far past 2.3 at this point :^) -------------------------------------------------------------------------------------- Steven Ludtke, Ph.D. ***@***.*** ***@***.***> ***@***.*** ***@***.***>>> Baylor College of Medicine Charles C. Bell Jr., Professor of Structural Biology Dept. of Biochemistry and Molecular Biology (www.bcm.edu/biochem <http://www.bcm.edu/biochem> <http://www.bcm.edu/biochem <http://www.bcm.edu/biochem>>) Academic Director, CryoEM Core (cryoem.bcm.edu <http://cryoem.bcm.edu/> <http://cryoem.bcm.edu/ <http://cryoem.bcm.edu/>>) Co-Director CIBR Center (www.bcm.edu/research/cibr <http://www.bcm.edu/research/cibr> <http://www.bcm.edu/research/cibr <http://www.bcm.edu/research/cibr>>) > On Apr 10, 2020, at 10:43 AM, John Bollinger ***@***.*** ***@***.***>> wrote: > > > Having recently engaged in the exercise of building the EMAN2 2.3 release from source, I had the opportunity to analyze compiler warnings to look for any that indicated genuine problems. There was a lot of noise, but a little signal, all of which appears still to apply to the current head. I'll be splitting that into four separate issues, of which this is the first. > > libEM/mrcio.cpp:206-207 is this: > > is_8_bit_packed = (mrch.mode == MRC_UHEX || > (mrch.imod_flags & 16) == 1 || double_nx); > But the condition (mrch.imod_flags & 16) == 1 never evaluates to true, because anything & 16 always evaluates to either 0 or 16. From code analysis alone, I suspect that (mrch.imod_flags & 16) != 0 would be consistent with the intention, but I am not sufficiently familiar with the details of the MRC format to confirm that. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub <#458 <#458>>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPKF2UHSA64MSRFTSPF7BLRL45BDANCNFSM4MFQZ32A <https://github.com/notifications/unsubscribe-auth/ACPKF2UHSA64MSRFTSPF7BLRL45BDANCNFSM4MFQZ32A>>. > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcryoem%2Feman2%2Fissues%2F458%23issuecomment-612103779&data=01%7C01%7CJohn.Bollinger%40StJude.org%7C7de538eef5cb4926404808d7dd6ad3cd%7C22340fa892264871b677d3b3e377af72%7C0&sdata=PtZQjJXUJi4Y%2F1svPyFH28drv8RVf09diXcwob15kgs%3D&reserved=0 <https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcryoem%2Feman2%2Fissues%2F458%23issuecomment-612103779&data=01%7C01%7CJohn.Bollinger%40StJude.org%7C7de538eef5cb4926404808d7dd6ad3cd%7C22340fa892264871b677d3b3e377af72%7C0&sdata=PtZQjJXUJi4Y%2F1svPyFH28drv8RVf09diXcwob15kgs%3D&reserved=0>>, or unsubscribe<https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA4A7UPNYKEODLQ4FJB4B5TRL5BF3ANCNFSM4MFQZ32A&data=01%7C01%7CJohn.Bollinger%40StJude.org%7C7de538eef5cb4926404808d7dd6ad3cd%7C22340fa892264871b677d3b3e377af72%7C0&sdata=05bjMWCk99G%2B6hdDAueiC4sTwgWdLV5TmREext2PCiY%3D&reserved=0 <https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAA4A7UPNYKEODLQ4FJB4B5TRL5BF3ANCNFSM4MFQZ32A&data=01%7C01%7CJohn.Bollinger%40StJude.org%7C7de538eef5cb4926404808d7dd6ad3cd%7C22340fa892264871b677d3b3e377af72%7C0&sdata=05bjMWCk99G%2B6hdDAueiC4sTwgWdLV5TmREext2PCiY%3D&reserved=0>>. ________________________________ Email Disclaimer: www.stjude.org/emaildisclaimer <http://www.stjude.org/emaildisclaimer> Consultation Disclaimer: www.stjude.org/consultationdisclaimer <http://www.stjude.org/consultationdisclaimer> — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#458 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACPKF2SZVXWFAIKUCBCGEDDRL5GHZANCNFSM4MFQZ32A>.