Error in symmetry.f90

[wsmorgan]

There seems to be an error in symmetry.f90's subroutine rm_3d_operations. This subroutine is supposed to remove the 3D operations from the point group for surface calculations. At the moment however, it leaves the 3D operations in. Looking back on the code it appears the difference came in when this operation was moved into it's own subroutine. At that point the following line:

       if (equal(sgrots(2:3,1,i),0._dp,eps) .and. equal(sgrots(1,2:3,i),0._dp,eps) .and.&
            & equal(sgrots(1,1,i),1._dp,eps)) then ! this operation is "2D"         

was changed to:

       if (equal(sgrots(2:3,1,i),0._dp,eps) .and. equal(sgrots(1,2:3,i),0._dp,eps) .and.&
            & equal(abs(sgrots(1,1,i)),1._dp,eps)) then ! this operation is "2D"         

If I reverse this change then the code removes the point group again as expected. The question is why was the change made and are we safe if we change it back?

[glwhart]

Who made the change? It won't leave in all of the 3D operations, but it will leave in some. I'd like to know what the log says. Someone had a reason...

[glwhart]

Well, it will leave in the 3D inversion for sure. Maybe none others. This would explain the factor of 2.

[wsmorgan]

Here is the message from the logs:

r67 | tobias | 2009-08-24 01:01:23 -0600 (Mon, 24 Aug 2009) | 6 lines

M    trunk/symmetry_module.f90
*-- added a routine "rm_3d_operations" that removes 3d operations.
    The code for this is taken from Gus's enumlib for surfaces.
    There, it is replaced by a call to this routine.

Looking back at the commit history shows for enumlib that this is also when the call to rm_3d_operations was placed in derivative_structures_generator.f90. At that point there was no abs() at this part of the code. Though there is nothing in the message to indicate why it Tobias added it.

[wsmorgan]

Changing this back to the way it was in the code makes the unit tests we have so for for enumlib pass 100% and all the celib tests pass except those the exist for rm_3d_operations. Just so we know.

[glwhart]

Tobias had a good reason, I think; something to do with doing surface CEs. It doesn't affect the enumeration, so I think we should find a way to work around the disagreement of the "pg" column in struct_enum.out during the unit tests.

[wsmorgan]

That is easy to do. We can just have it ignore the point group when it does the comparisons.

Wiley Morgan http://hagrid.byu.edu/Wiley#Bio

On Thu, Apr 14, 2016 at 3:50 PM, Gus Hart notifications@github.com wrote:

Tobias had a good reason, I think; something to do with doing surface CEs. It doesn't affect the enumeration, so I think we should find a way to work around the disagreement of the "pg" column in struct_enum.out during the unit tests.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/msg-byu/symlib/issues/8#issuecomment-210167937

[glwhart]

In other words, the addition of the abs doesn't make the enumerations wrong (as far as we can tell), and I think Tobias needed it this way for surface CEs, so let's leave it in and find a way to fix the unit tests.

[wsmorgan]

So I just changed the template for the unit tests to have it ignore the point group as a key that it uses to compare structures. The unit tests now pass with the abs() still in.