Origin choice for selected space groups #711
Conversation
|
Hello @saransh13! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2024-08-20 20:43:25 UTC |
| t = 'a' + t # get the translation without any rotation | ||
| sym = np.squeeze(SYM_fillgen(t, sgn=-1)) | ||
| sym2 = np.squeeze(SYM_fillgen(t)) | ||
| for i in range(1, genmat.shape[0]): | ||
| generator = np.dot(sym2, np.dot( | ||
| np.squeeze(genmat[i, :, :]), | ||
| sym)) | ||
| frac = np.modf(generator[0:3, 3])[0] | ||
| frac[frac < 0.] += 1. | ||
| frac[np.abs(frac) < 1E-5] = 0.0 | ||
| frac[np.abs(frac-1.0) < 1E-5] = 0.0 | ||
| generator[0:3, 3] = frac | ||
| genmat[i, :, :] = generator |
There was a problem hiding this comment.
@saransh13 This is the only section with functional code changes, correct?
There was a problem hiding this comment.
This only affects materials with a different origin choice. What material is used in the failing test?
… only assume setting 1
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #711 +/- ##
=======================================
Coverage 33.49% 33.50%
=======================================
Files 129 129
Lines 21237 21253 +16
=======================================
+ Hits 7113 7120 +7
- Misses 14124 14133 +9 ☔ View full report in Codecov by Sentry. |
Issue #559 pointed out that the density of diamond was 2x the theoretical density. It was traced back to the default choice of the origin.
hexrdhad origin choice 1 by default, while most CIF files available online assume origin choice 2.The following changes have been made in this PR:
h5file I/O, the origin choice can be explicitly passed as an argumentsgsetting. This will override the value from the file.Fixes #559 on the hexrd side. The GUI still needs to implement an input for the origin choice.