Expose spg_get_symmetry_from_database in the Fortran interface#530
Expose spg_get_symmetry_from_database in the Fortran interface#530LecrisUT merged 2 commits intospglib:developfrom
spg_get_symmetry_from_database in the Fortran interface#530Conversation
LecrisUT
left a comment
LecrisUT
left a comment
There was a problem hiding this comment.
I guess it's fine. It's not ideal that you need to do the transpose manually on the Fortran side, but it's the fastest way around it.
LecrisUT
changed the title
spg_get_symmetry_from_database in the Fortran interface
|
One final change if you may, between the lines: Lines 12 to 14 in d9c572d Can you add a small snippet like: ### Fortran API
- Expose `spg_get_symmetry_from_database`. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #530 +/- ##
========================================
Coverage 84.12% 84.12%
========================================
Files 26 26
Lines 8122 8122
Branches 1702 1698 -4
========================================
Hits 6833 6833
Misses 1289 1289
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Will do. Probably not today as I am fully booked until 8 pm (9 pm CET).
I will check for the matrix transposition. I did not realize they were
transposed in the dataset. in my current implementation of spglib in CP2K,
I am using spg_get_symmetry, so it seemed the most natural to use as a
comparison. As I intend to make full use of the dataset, the matrix
transposition is a critical issue. Thanks for pointing this out. I will
update spg_get_symmetry_from_database accordingly.
…On Wed, Oct 2, 2024 at 4:13 PM Cristian Le ***@***.***> wrote:
One final change if you may, between the lines:
https://github.com/spglib/spglib/blob/d9c572d1986b0eccc4ed5f18c8d827729a320313/ChangeLog.md?plain=1#L12-L14
Can you add a small snippet like:
### Fortran API
- Expose `spg_get_symmetry_from_database`.
—
Reply to this email directly, view it on GitHub
<#530 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDSCPDIUNDEESV273CYNQLZZQETDAVCNFSM6AAAAABPH32OV2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBYHEZTQMJZG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
For example |
|
Hi Cristian and Atsushi,
Unless the c_f_pointer function from iso_c_binding transposes the matrices,
it seems to me that the rotation and translation matrices in the dataset
are in the same arrangement as in spg_get_symmetry and the new function I
created:
Here from the dataset module
call c_f_pointer(dset_c%rotations, rotations, shape=[3, 3, n_operations])
Here from the function I added
integer(c_int), intent(out) :: rotation(3, 3, *)
The tests I made with both my new function and the dataset module give the
same results for the rotations. In both cases, the rotation matrices are
transposed compared to their C counterparts. I compared them with the
rotation matrices contained in the Python dataset.
The function I added should therefore be fine as it is consistent with
other modules/functions in the Fortran interface.
Regards,
Pierre
…On Thu, Oct 3, 2024 at 3:34 AM Atsushi Togo ***@***.***> wrote:
it seemed the most natural to use as a comparison.
For example spg_get_symmetry, too, the data is just passed without
rearranging the data order in memory.
—
Reply to this email directly, view it on GitHub
<#530 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABDSCPGHMTIXJEBLFYFFSG3ZZSUJRAVCNFSM6AAAAABPH32OV2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJQGM3TQOBZGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Indeed, consistency wise it is fine, it's just the usability point it is not ideal because we are putting a burden on the user to do such transposition. This is of course something that should be handled by us more generally. I will finally have more time as my contract ends, and I'll come back to contribute more to the project and provide a more modern object orientated interface. For this PR, it's only the |
|
I added the comment in the Changelog.md as you asked. I agree with you that, ideally, the end users should not have to think about the array arrangement within the library. That said, it is not always bad to have the matrices already transposed, depending on the linear algebra operations done with them. Thanks for your help. |
|
@lan496 can you overwrite the test requirement? I am not sure why F39 failed there, but it will be EOL in a few weeks, so you can just those test requirements. |
|
@LecrisUT |
4 participants
I added a simple Fortran wrapper for spg_get_symmetry_from_database based on spg_get_symmetry wrapper.