arohl / gdis

A visualization program for the display, manipulation, and analysis of isolated molecules and periodic structures
GNU General Public License v2.0
43 stars 17 forks source link

Add extra functionality to Model:Editing data #43

Closed cajfisher closed 3 years ago

cajfisher commented 3 years ago

Functions added to display and modify charges and masses of individual atoms and groups of atoms (selected by user) in the Model: Editing sidebar (many structure files include mass values which may or may not be the same as the standard database values, for example). Option added to Model Editing->Builder dialogue to allow shells to be deleted. Also fixed Invert function so hidden atoms are not selected.

ovhpa commented 3 years ago

Hello, and thank you for your contribution. I think I find a few problems. At analysis step:

  1. the following is reported in static analysis (and valgrind):

    Description: Value stored to 'charge' is never read
    File: src/gui_edit.c
    Line: 2875

    The variable is un-necessarily initialized in:

    charge = g_strdup("");

    I suggest removing the line. This is also reported by valgrind because it is a (very small) memory leak.

  2. The files edit.c and select.c are both using the model_content_refresh function, which is undefined:

    warning: implicit declaration of function ‘model_content_refresh’

    I suggest including the relevant source header ( model.h ? ).

At functional level:

  1. there is a problem in which the values of the Model : Editing field are not updated on selection. Step to reproduce: open a file with gdis: (from the src directory, after build)
    ../bin/gdis ../models/vasprun.xml

    then switch to Model : Editing and select some atoms (here CO) previously: (the X, Y, and Z data is updated) before

now: (the data is not updated, one needs to select another field, for ex. Model : Content, then switch back to Model : Editing to see the updated X, Y, Z, Charge, and Mass data) after

(I have not yet review more than that)

Best,

cajfisher commented 3 years ago

Sorry about the careless bugs. All fixed now!


差出人: Okadome Valencia @.> 送信日時: 2021年7月15日 11:38 宛先: arohl/gdis @.> CC: クレイグ・フィッシャー @.>; Author @.> 件名: Re: [arohl/gdis] Add extra functionality to Model:Editing data (#43)

Hello, and thank you for your contribution. I think I find a few problems. At analysis step:

  1. the following is reported in static analysis (and valgrind):

Description: Value stored to 'charge' is never read

File: src/gui_edit.c

Line: 2875

The variable is un-necessarily initialized in:

charge = g_strdup("");

I suggest removing the line. This is also reported by valgrind because it is a (very small) memory leak.

  1. The files edit.c and select.c are both using the model_content_refresh function, which is undefined:

warning: implicit declaration of function ‘model_content_refresh’

I suggest including the relevant source header ( model.h ? ).

At functional level:

  1. there is a problem in which the values of the Model : Editing field are not updated on selection. Step to reproduce: open a file with gdis: (from the src directory, after build)

../bin/gdis ../models/vasprun.xml

then switch to Model : Editing and select some atoms (here CO) previously: (the X, Y, and Z data is updated) [before]https://user-images.githubusercontent.com/36496189/125719021-a6807c90-8c1b-4792-9297-48c3249419c2.png

now: (the data is not updated, one needs to select another field, for ex. Model : Content, then switch back to Model : Editing to see the updated X, Y, Z, Charge, and Mass data) [after]https://user-images.githubusercontent.com/36496189/125719028-a100a5b9-6ebc-4574-b9f8-b29bcbc180f0.png

(I have not yet review more than that)

Best,

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/arohl/gdis/pull/43#issuecomment-880343833, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AP5N32H3MLGRG5DBYL7ZUP3TXZCZXANCNFSM5AGHDNVA.

ovhpa commented 3 years ago

Hello and thanks again for your contributions,

Sorry for the "radio silence" of past month, but I am currently changing affiliation, after some scary unemployment time. And even though I remain in the same university, the paperwork is as heavy as if it was my first time there...

Anyway, I am now back ;)

I found nothing strange from this PR so I will merge it. I will examine the other 2 PR later this week..

While I was "away" someone report me a strange behavior: it seems the background color can no longer be changed (the changes won't be recorded in next gdis run as well). One uneasy workaround is to make background change in the gdisrc file. When looking more into it, it seems colors can't be changed at all on elements too (on a ubuntu 20.04 newly installed machine). @cajfisher @arohl did you notice this as well? I will look into this issue after I finish looking into the pull requests ;)

Best,