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

Demet #2317

Closed
wants to merge 7 commits into from
Closed

Demet #2317

wants to merge 7 commits into from

Conversation

recohen
Copy link

@recohen recohen commented Feb 24, 2020

Please review the developer documentation
on the wiki of this project that contains help and requirements.

Proposed changes

Added internal energy and smearing energy to analysis for pwscf in nexus
Describe what this PR changes and why. If it closes an issue, link to it here
with a supported keyword.

What type(s) of changes does this code introduce?

Delete the items that do not apply

  • New feature

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

tomcat3

Checklist

Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.

  • No. This PR is up to date with current the current state of 'develop'
    No. Code added or changed in the PR has been clang-formatted
    No. This PR adds tests to cover any new code, or to catch a bug that is being fixed
    No. Documentation has been added (if appropriate)

@qmc-robot
Copy link

Can one of the admins verify this patch?

@prckent prckent requested a review from jtkrogel February 24, 2020 17:04
@jtkrogel
Copy link
Contributor

NB: it will take me a bit to get to this as I am (and others are) currently on a steep ramp for both proposal writing and March meeting.

@recohen
Copy link
Author

recohen commented Feb 26, 2020 via email

Copy link
Contributor

@jtkrogel jtkrogel left a comment

Choose a reason for hiding this comment

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

@recohen I've had a chance to review your updates. Mostly things look OK, but there are a few places where deleted code should be restored and minor formatting issues should be resolved.

@@ -0,0 +1,52 @@
#! /usr/bin/env python
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is an executable. Please move to nexus/bin.

@@ -0,0 +1,52 @@
#! /usr/bin/env python
Copy link
Contributor

Choose a reason for hiding this comment

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

Also an executable. Move to nexus/bin.

# equilibration and autocorrelation time specified
#print pa.md_statistics(equil=20,autocorr=45)
#end if
pa.md_plots(filename,int(begin),removeskips=removeskips,startfromone=startfromone)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add #end def closure.

# equilibration and autocorrelation time specified
#print pa.md_statistics(equil=20,autocorr=45)
#end if
pa.md_plots(filename,int(begin),removeskips=removeskips,startfromone=startfromone)
Copy link
Contributor

Choose a reason for hiding this comment

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

Add #end def closure.

self.md_data = md
self.md_stats = self.md_statistics()
return
#end if
Copy link
Contributor

Choose a reason for hiding this comment

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

Add #end def closure.

@@ -242,17 +249,6 @@ def analyze(self):
read_rel = False
for i in xrange(len(lines)):
l = lines[i]
Copy link
Contributor

Choose a reason for hiding this comment

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

Restore bands processing.

data_file = os.path.join(datadir,cont.prefix+'.xml')
#end if
data = read_qexml(data_file)
data = read_qexml(os.path.join(datadir,'data-file.xml'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Restore alternate xml data path.

@@ -930,6 +868,85 @@ def plot_bandstructure(self, filename=None, filepath=None, max_min_e = None, sho
show()
#end if
#end def plot_bandstructure
def md_statistics(self,equil=None,autocorr=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Add #end closures.

@@ -930,6 +868,85 @@ def plot_bandstructure(self, filename=None, filepath=None, max_min_e = None, sho
show()
#end if
#end def plot_bandstructure
def md_statistics(self,equil=None,autocorr=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

md_statistics and md_plots appear to be repeated with some variations in a few files. Is there a good way to consolidate and avoid duplication?

@@ -906,7 +906,6 @@ class ee(Section):
#end for


exit_ = exit
Copy link
Contributor

Choose a reason for hiding this comment

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

Restore alias for exit function.

@jtkrogel
Copy link
Contributor

jtkrogel commented Apr 2, 2020

Note: the updates here should be checked against the test added in #2362.

@prckent
Copy link
Contributor

prckent commented Aug 9, 2023

Closing this as stale. Would be happy to review a freshened and up to date version. I would add any utilities separately from new machine support. An example of adding a new machine that was merged is at #4598 .

@prckent prckent closed this Aug 9, 2023
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.

4 participants