mcodev31 / libmsym

molecular point group symmetry lib
MIT License
75 stars 32 forks source link

enhanced the python binding by adding some missing class and better string representation #37

Open kylincaster opened 11 months ago

kylincaster commented 11 months ago

Dear all,

I have made some modifications to the libmsym Python binding by adding the missing class, such as subgroup, and providing a more enhanced string representation.

In the element.c file, I have adjusted the code to allow for unknown element names, with a maximum length of 1024 extra unknown elements. This means that atomic elements like "MX" are now permitted in libmsym.

Your sincerely, Kylin

mcodev31 commented 11 months ago

Hi,

Thanks for the pull request.

I'm not quite finished with looking through all the python code, but figured I'd give some quick feedback. atomic_orbital_symbols would need to have more orbitals defined, the find_misc function feels like it's part of a specific project (given it e.g. symmetrizes the molecule which I'm sure not all would want out of a find_x api) the fix to the @l/m.setter fix doesn't fix the actual problem that it assigns the wrong member (although nice that you found it).

There is already support for custom elements in libmsym, you just have to define all the properties, the element.c modification is for complementing data when it is missing. Also the static allocation of extra_elements means that if you were to use more than 1 context they would override data for eachother.

This patch will add a molecule with 4 custom elements to the example rather than the file input

diff --git a/examples/example.c b/examples/example.c
index 89e5e84..c88c5d4 100644
--- a/examples/example.c
+++ b/examples/example.c
@@ -15,6 +15,7 @@
 #include "example.h"

 int read_xyz(const char *name, msym_element_t **ratoms);
+int custom(msym_element_t **ratoms);

 #ifndef __LIBMSYM_NO_VLA__
 void printSALC(msym_salc_t *salc, msym_element_t *melements){
@@ -69,7 +70,8 @@ int example(const char* in_file, msym_thresholds_t *thresholds){
     /* This function reads xyz files.
      * It initializes an array of msym_element_t to 0,
      * then sets the coordinates and name of the elements */
-    int length = read_xyz(in_file, &elements);
+    int length = custom(&elements);
+
     if(length <= 0) return -1;

@@ -451,6 +453,8 @@ int example(const char* in_file, msym_thresholds_t *thresholds){
     myelement.v[0] = melements[0].v[0];
     myelement.v[1] = melements[0].v[1];
     myelement.v[2] = melements[0].v[2];
+    memcpy(myelement.name,melements[0].name,sizeof(myelement.name));
+       myelement.m = melements[0].m;

     printf("\nWould you like generate a new molecule based on %s at [%lf %lf %lf]? [y/N]:", melements[0].name, melements[0].v[0], melements[0].v[1], melements[0].v[2]);
     if(scanf(" %c", &yn) > 0 && (yn | 0x60) == 'y'){
@@ -500,6 +504,19 @@ err:
     return ret;
 }

+int custom(msym_element_t **ratoms) {
+       msym_element_t arr[4] = {
+               [0] = {.m = 1000.0, .v = { 1, 1, 1}, .n = 1000.0, .name = "mx"},
+               [1] = {.m = 1000.0, .v = { 1,-1,-1}, .n = 1000.0, .name = "mx"},
+               [2] = {.m = 1000.0, .v = {-1, 1,-1}, .n = 1000.0, .name = "mx"},
+               [3] = {.m = 1000.0, .v = {-1,-1, 1}, .n = 1000.0, .name = "mx"}
+       };
+       msym_element_t *a = malloc(sizeof(arr));
+       memcpy(a,arr,sizeof(arr));
+       *ratoms = a;
+       return 4;
+}
+
 int read_xyz(const char *name, msym_element_t **ratoms) {
     FILE *fp = fopen(name,"r");
     msym_element_t *a;

The

kylincaster commented 11 months ago

Thank you for your detailed feedback on my modifications. My goal is to create a user-friendly Python interface find_misc. I plan to enhance the Python bindings further.

By the way, do you have any plans to implement SALC symmetry for point groups with complex characters? I attempted it but faced challenges. Your insights would be appreciated.