Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kylincaster
Copy link

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
Copy link
Owner

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants