Conversation
…ic_axes array. Caused segfaults.
|
Update: Using an online tool, if I cut spacegroup 38, I get layergroup 35. Thus, if I change (lg 35 -> sg 35, to lg 35-> sg 38), along with the tolerance update: all 15733 materials pass the sanity check. With these changes, the data is here: |
|
Thanks @mikaelkuisma1 for your bug fix along the detailed explanation (helpful!), and systematic test. How did you run over C2DB 15000 data systematically? Currently I am busy. I would like to have time to look at it next week or later. But I have not yet been very familiar with the layer group, and so I need also some basic study on it, too. @site-g, can you have a look at it? Probably we need to refactor the code, too. @lan496, do you have any comment? (also magnetic layer group for the future) |
|
|
(Just a comment) For testing, we may need another repository to check layer-group implementations. For example, I've created and checked magnetic-space-group implementations for MAGNDATA in a private repo. For refactoring, wrapper functions to hide comparison of |
Codecov ReportBase: 85.12% // Head: 85.12% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## develop #199 +/- ##
========================================
Coverage 85.12% 85.12%
========================================
Files 16 16
Lines 1318 1318
========================================
Hits 1122 1122
Misses 196 196 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
That sounds very helpful for avoiding typo! In fact, the layer-group implementation is far from being completed. There requires:
And also some codes need to be refactored, but I have being very busy since the end of last year and contribute nothing to the project. Very sorry. |
|
@site-g, no need to be sorry. Just providing some comments is helpful. So can I merge this PR? Then we will discuss on the layer group in a new issue. |
|
I think we can, this is a typo. |
|
Merged. Thanks @mikaelkuisma1. |

I run the layer group detection to all 15000 2D-materials in C2DB. This resulted in 22 Bus errors or segfaults.
Here is a simple script modified from example.c to reproduce the problem.
gcc runlayers.c -I../include -L../_build -O0 -lsymspg -fsanitize=address -static-libasan -ggdb && LD_LIBRARY_PATH=
pwd/../_build ./a.outI traced the error down to
cel_layer_is_overlapfunction, which typically had periodic axes 0, 1, until they were 2 1070703038, and the code crashed. It is quite miraculous that the code worked before, probably due to some stack alignment of previous calls, that the periodic axes happened to be 0 and 1.This merge request allows calculation for those 22 systems, while keeping all other results the same. However, I am still not sure are the results correct.
There are some more bugs remaining still. Edit: all of these might be resolved, see comment below.
periodic axes are hard coded in some places, however, I do not know if this is related
(Mapped according to this table) https://arxiv.org/pdf/1306.1280.pdf
Here LG is layergroup, and SG is spacegroup, and mappedsg is mapping of layer group to spacegroup by the table.
https://pastebin.com/sUqCtu4j
Most of them are predicted to be layergroup 35, according to article, spacegroup 35, but predicted to be spacegroup 38.