Discussion List Archives

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Imgcif-l] CBFlib_0.7.7 update to subrelease 0.7.7.4

Dear Herbert,

I confirm that your patch works for compiling the code and that it seems 
to fail in the same place, on 3D arrays only. No idea what goes wrong, 
but there are two failures as below (including 1, 41, 50?). Perhaps it 
is worth trying to pack the array indices themselves as data by using 
some prime numbers, eg:
    KFRAME( I, J, K ) = 3*I + 5*J + 7*K
These might help to differentiate transpose problems from corrupted data 
more easily, as I think you can get the array indices back by factorising.

Best

Jon
---


[...]
echo testflatout.cbf |  ./bin/test_fcb_read_image  > 
test_fcb_read_testflatout.out
diff test_fcb_read_testflatout.out test_fcb_read_testflatout_orig.out
1,1608c1,7
<   NAME OF TEST CBF
<   1000 x 1000 I4 TEST
<   1000 x 1000 I2 TEST
<   1000 x 1000 I4 TEST, WITH -3 on diag and transpose
<   1000 x 1000 I2 TEST, WITH -3 on diag and transpose
<   50 x 60 x 70 3D_I4 TEST
<  KFRAME( 40 , 40 , 50 ) =  1000 , SHOULD BE  -3
<  KFRAME( 1 , 41 , 50 ) =  1000 , SHOULD BE  89
<  KFRAME( 41 , 41 , 50 ) =  1000 , SHOULD BE  -3
[...]

echo testflatpackedout.cbf |  ./bin/test_fcb_read_image > 
test_fcb_read_testflatpackedout.out
diff test_fcb_read_testflatpackedout.out 
test_fcb_read_testflatpackedout_orig.out
1,53732c1,7
<   NAME OF TEST CBF
<   1000 x 1000 I4 TEST
<   1000 x 1000 I2 TEST
<   1000 x 1000 I4 TEST, WITH -3 on diag and transpose
<   1000 x 1000 I2 TEST, WITH -3 on diag and transpose
<   50 x 60 x 70 3D_I4 TEST
<  KFRAME( 43 , 50 , 49 ) =  624 , SHOULD BE  1000
<  KFRAME( 44 , 50 , 49 ) =  577 , SHOULD BE  1000
<  KFRAME( 45 , 50 , 49 ) =  571 , SHOULD BE  1000
<  KFRAME( 46 , 50 , 49 ) =  570 , SHOULD BE  1000

Herbert J. Bernstein wrote:
> Dear Jon,
> 
>    I have most of what is needed for g95 ready to go, but there
> is one puzzling problem left -- when working with the 3d array
> test cases, as in your report, I am getting what seems to be
> misindexing for 40,40,50 and up in an array that is 50x60x70.
> The 2d cases work like a champ, and the 3d test works up to that
> point.  Any compiler option suggestions before I revert to manual
> indexing?
> 
>    Regards,
>      Herbert
> 
> P.S.  I had to change your fix to fcb_read_bits.m4 to keep it
> working with gfortran as well as g95.  Here is the updated patch:
> 
> *** fcb_read_bits.m4      Sun Apr  1 22:12:49 2007
> --- fcb_read_bits.m4    Mon May  7 20:28:57 2007
> ***************
> *** 17,23 ****
>          INTEGER,      INTENT(IN):: LINT
>          INTEGER(4),  INTENT(OUT):: IINT(LINT)
>          INTEGER                    I,J,LBITCOUNT,COUNT,KINTS
> !       INTEGER(8)                 BITCODE,TBITCODE, M
>    !-----------------------------------------------------------------------
> 
>          INTEGER                    MAXBITS, NUMINTS
> --- 17,23 ----
>          INTEGER,      INTENT(IN):: LINT
>          INTEGER(4),  INTENT(OUT):: IINT(LINT)
>          INTEGER                    I,J,LBITCOUNT,COUNT,KINTS
> !       INTEGER(8)                 BITCODE,TBITCODE, M, MASK8
>    !-----------------------------------------------------------------------
> 
>          INTEGER                    MAXBITS, NUMINTS
> ***************
> *** 26,37 ****
>    `
>          MAXBITS = 32
>          NUMINTS = (BITCOUNT+MAXBITS-1)/MAXBITS
>         
>          DO KINTS = 1,NUMINTS
>            LBITCOUNT = MAXBITS
>            IF (KINTS.EQ.NUMINTS) LBITCOUNT = BITCOUNT-(NUMINTS-1)*32
>            COUNT = BCOUNT
> !         BITCODE = IAND(BBYTE,Z''`000000FF''`)
>            DO
>              IF (COUNT .GE. LBITCOUNT) EXIT
>              BYTE_IN_FILE=BYTE_IN_FILE+1
> --- 26,39 ----
>    `
>          MAXBITS = 32
>          NUMINTS = (BITCOUNT+MAXBITS-1)/MAXBITS
> +       MASK8 = Z''`000000FF''`
>         
>          DO KINTS = 1,NUMINTS
>            LBITCOUNT = MAXBITS
>            IF (KINTS.EQ.NUMINTS) LBITCOUNT = BITCOUNT-(NUMINTS-1)*32
>            COUNT = BCOUNT
> !         BITCODE = BBYTE
> !         BITCODE = IAND(BITCODE,MASK8)
>            DO
>              IF (COUNT .GE. LBITCOUNT) EXIT
>              BYTE_IN_FILE=BYTE_IN_FILE+1
> ***************
> *** 40,46 ****
>                REC_IN_FILE,BYTE_IN_FILE,BBYTE)
>              IF (FCB_READ_BITS.NE.0) RETURN
>                BCOUNT=8
> !             TBITCODE = IAND(BBYTE,Z''`000000FF''`)
>                CALL MVBITS(TBITCODE,0,MIN(8,32-COUNT),BITCODE,COUNT)
>                COUNT = COUNT+8
>            END DO
> --- 42,49 ----
>                REC_IN_FILE,BYTE_IN_FILE,BBYTE)
>              IF (FCB_READ_BITS.NE.0) RETURN
>                BCOUNT=8
> !             TBITCODE = BBYTE
> !             TBITCODE = IAND(TBITCODE,MASK8)
>                CALL MVBITS(TBITCODE,0,MIN(8,32-COUNT),BITCODE,COUNT)
>                COUNT = COUNT+8
>            END DO
> ***************
> *** 62,68 ****
>            ! SAVE THE REMAINING BITS FOR NEXT TIME
>         
>            TBITCODE = BBYTE
> !         TBITCODE = 
> ISHFT(IAND(TBITCODE,Z''`000000FF''`),-(BCOUNT-(COUNT-LBITCOUNT)) )
>            BBYTE = TBITCODE
>            BCOUNT = COUNT-LBITCOUNT
>         
> --- 65,71 ----
>            ! SAVE THE REMAINING BITS FOR NEXT TIME
>         
>            TBITCODE = BBYTE
> !         TBITCODE = ISHFT(IAND(TBITCODE,MASK8),-(BCOUNT-(COUNT-LBITCOUNT)) )
>            BBYTE = TBITCODE
>            BCOUNT = COUNT-LBITCOUNT
> 
> 
> 
> At 1:09 PM -0400 5/7/07, Herbert J. Bernstein wrote:
>> Dear Jon,
>>
>>   Thank you.  This is very helpful, and it is a good idea to send it
>> to the list to save others from having to report the same problems.
>> Since windows is a very odd case, I think I'll make a distinct
>> Makefile_MINGW, rather than use a make.inc  I'll wait a couple of
>> days for other trouble reports and put out an update with your
>> changes early next week.
>>
>>   Regards,
>>     Herbert
>>
>> At 6:29 PM +0200 5/7/07, Jon Wright wrote:
>>> Dear Herbert,
>>>
>>> Here are some notes for compiling with the mingw system on windows. 
>>> It is a free gnu c compiler with "msys" bash shell.  For f90 I used 
>>> the g95 compiler ( www.g95.org ).
>>>
>>> g95 is fussy about the inline binary constants. A diff of 
>>> fcb_read_bits.m4 should be attached - it could easily contain 
>>> errors as I don't really know the intentions of the code. I have 
>>> just used the transfer intrinsic to give a matching type to use in 
>>> IAND.
>>>
>>> libmap.a is required for target 'all' in the Makefiles but my 
>>> 'make' doesn't see how to make it. I've attached an edited makefile 
>>> for this compiler / platform. There are some issues with "time" not 
>>> existing and, no directories /usr/bin etc. Might be worthwhile to 
>>> have a Makefile and then a platform dependent make.inc ? I had to 
>>> change in lots of places.
>>>
>>> My favourite windows C compiler (mingw gcc) didn't know about 
>>> bzero: needed to add something near the top of cbf_codes.c:
>>>
>>> #ifdef __MINGW32__
>>> #define bzero(ptr,size) memset (ptr, 0, size);
>>> #endif
>>>
>>>
>>> The needed to remove the -ansi flag from the makefile as "swab" 
>>> function is otherwise hidden by an ifdef in <string.h>.
>>>  src/cbf_uncompressed.c:431: warning: implicit declaration of function `swab'
>>>
>>> mkstemp is missing on mingw32 also (in img2cif.c etc), I have 
>>> attached a header file implementation, but I am not sure if it 
>>> works. If this is in the example directory then add this to 
>>> img2cif.c, cif2cbf.c and convert_minicbf.c:
>>>
>>> #ifdef __MINGW32__
>>> #include "mkstemp.h"
>>> #endif
>>>
>>> It might be better to use tmpfile(?). /tmp exists on my system, but 
>>> is extremely unlikely to be there in general. This change allows 
>>> the programs to compile, but not pass the tests. It seems windows 
>>> (mingw MSYS bash shell) does like reading or writing binary images 
>>> through stdin or stdout. I changed the makefile to use filenames 
>>> for the images. This means the mkstemp supplied may be no good. It 
>>> might be better to force stdin and stdout to be binary streams in 
>>> this case, as in 
>>> (http://archives.postgresql.org/pgsql-hackers-win32/2005-01/msg00227.php)
>>>
>>> I can investigate further if you think it is likely that windows 
>>> users will need this functionality.
>>>
>>> Since "time" is not a windows/dos command - I added a makefile 
>>> variable $(TIME) which is empty for this system.
>>>
>>> Finally, I get a slew of test failures which make me worry I got 
>>> the fortran wrong, trimmed output from "make tests" is attached. 
>>> With the fixes it appears to be OK apart from the map part.
>>>
>>> I shall try to look at the python bindings again soon. Leaving 
>>> tomorrow for the APS user meeting  for a week and then the UK for 
>>> the week after.
>>>
>>> Best wishes,
>>>
>>> Jon
>>>
> 

_______________________________________________
imgcif-l mailing list
imgcif-l@iucr.org
http://scripts.iucr.org/mailman/listinfo/imgcif-l

Reply to: [list | sender only]
International Union of Crystallography

Scientific Union Member of the International Science Council (admitted 1947). Member of CODATA, the ISC Committee on Data. Partner with UNESCO, the United Nations Educational, Scientific and Cultural Organization in the International Year of Crystallography 2014.

International Science Council Scientific Freedom Policy

The IUCr observes the basic policy of non-discrimination and affirms the right and freedom of scientists to associate in international scientific activity without regard to such factors as ethnic origin, religion, citizenship, language, political stance, gender, sex or age, in accordance with the Statutes of the International Council for Science.

Useful links