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

Report changes #98

Merged
merged 14 commits into from
Jul 6, 2021
Merged

Report changes #98

merged 14 commits into from
Jul 6, 2021

Conversation

rlavendomme
Copy link
Collaborator

Small additions/fixes to the reported values.

Note that the volumes including outside space are tagged in the UI, calculation report file and command line output but the change was not tested in the command line output.

rlavendomme and others added 5 commits July 2, 2021 16:28
Automatic export output file now include the name of input structure file.
As mentioned in issue #88 : 
When not analyzing a crystal unit cell, in single probe mode, the first cavity includes outside space without any meaningful boundary. Therefore the reported volume is meaningless. Consequently, the total probe core volume is also meaningless.
Similarly, in two probes mode, the total large probe core volume is meaningless.

This information is now clearly given in the UI and report output file.
@rlavendomme rlavendomme requested a review from jmaglic July 4, 2021 11:33
Copy link
Member

@jmaglic jmaglic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo it's better to leave out the "not meaningful" values altogether. Why keep them if they're not meaningful?
See my comment about file names.

src/model_outputfiles.cpp Outdated Show resolved Hide resolved
src/base_guicontrol.cpp Outdated Show resolved Hide resolved
src/base_guicontrol.cpp Outdated Show resolved Hide resolved
src/misc.cpp Outdated Show resolved Hide resolved
jmaglic and others added 3 commits July 4, 2021 19:20
Remove the volume values that are meaningless because they include an unlimited outside space.
Add missing line breaks.
@rlavendomme rlavendomme requested a review from jmaglic July 4, 2021 18:55
Copy link
Member

@jmaglic jmaglic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change phrasing to: "No physical meaning, contains all volume outside the structure"

@rlavendomme rlavendomme requested a review from jmaglic July 5, 2021 21:41
@jmaglic jmaglic merged commit 2bddd11 into master Jul 6, 2021
@jmaglic jmaglic deleted the report-changes branch July 6, 2021 07:06
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