-
Notifications
You must be signed in to change notification settings - Fork 16
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
Saltproc capability addition and UI improvement #29
Conversation
Here you go with the Pull Request ! The fixes are suggested by autopep8. |
Fix pep8 errors
saltproc/saltproc.py
Outdated
if key == self.driver_mat_name: | ||
key = 'driver' | ||
elif key == self.blanket_mat_name: | ||
key = 'blanket' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these four lines copy pasted from above? Maybe they should be their own function.
Never copy paste. Use the DRY principle (Don't Repeat Yourself)
saltproc/saltproc.py
Outdated
|
||
if restart: | ||
# resize dataset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it needs a comment to be readable, then this should probably be its own function.
saltproc/saltproc.py
Outdated
@@ -374,6 +383,7 @@ def reopen_db(self, restart): | |||
# set past time | |||
# !! this time thing should be made certain | |||
self.current_step = np.amax(np.nonzero(self.keff)) + 1 | |||
self.current_step = np.amax(np.nonzero(sum(self.driver_before_db))) + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you're setting a variable equal to something and then set it equal to something else in the next line.
Delete one of the two. Ideally, simplifying the syntax.
saltproc/saltproc.py
Outdated
@@ -392,8 +402,11 @@ def reopen_db(self, restart): | |||
|
|||
# write new material file | |||
self.core = {} | |||
self.core[self.driver_mat_name] = self.driver_after_db[self.current_step - 1] | |||
self.core[self.blanket_mat_name] = self.blanket_after_db[self.current_step - 1] | |||
self.core[self.driver_mat_name] = self.driver_after_db[self.current_step - 2] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why hardcode this number 2?
saltproc/saltproc.py
Outdated
self.core[self.blanket_mat_name] = self.blanket_after_db[self.current_step - 1] | ||
self.core[self.driver_mat_name] = self.driver_after_db[self.current_step - 2] | ||
self.core[self.blanket_mat_name] = self.blanket_after_db[self.current_step - 2] | ||
print(self.current_step - 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why print this?
saltproc/saltproc.py
Outdated
self.core[self.blanket_mat_name] = self.blanket_after_db[self.current_step - 2] | ||
print(self.current_step - 1) | ||
print(self.core[self.driver_mat_name]) | ||
print(self.core[self.blanket_mat_name]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these print statements for debugging or runtime logging? If the former, I recommend pdb. If the latter, I recommend adding a logging system and .log file output to saltproc.
saltproc/saltproc.py
Outdated
@@ -461,13 +474,13 @@ def read_dep(self, boc=False): | |||
indx = z.index('%') | |||
mdens = z[indx-1] | |||
if boc: | |||
mdens = z[indx-2] | |||
mdens = z[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
saltproc/saltproc.py
Outdated
@@ -493,8 +506,9 @@ def write_mat_file(self): | |||
(self.current_step, ana_keff_boc[0], ana_keff_boc[1], | |||
ana_keff_eoc[0], ana_keff_eoc[1])) | |||
for key, val in self.core.items(): | |||
matf.write(self.mat_def_dict[key].replace( | |||
'\n', '') + ' fix 09c 900\n') | |||
if key == '': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if key was never initialized? It might be a None type.
saltproc/mcsfr/run_saltproc.py
Outdated
# False: Start from timestep zero. | ||
restart = False | ||
|
||
# True: Uses blue water command (aprun) to run SERPERNT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you mean "Blue Waters"
saltproc/mcsfr/run_saltproc.py
Outdated
############################################################## | ||
|
||
# SERPENT input file | ||
input_path = os.path.dirname(os.path.abspath(__file__)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line will confuse the user. It shouldn't be edited, but it's the first line in the section that is supposed to be for setting variables.
saltproc/mcsfr/run_saltproc.py
Outdated
input_path = os.path.dirname(os.path.abspath(__file__)) | ||
sys.path.append(input_path) | ||
print(input_path) | ||
input_file = os.path.join(input_path, 'mcsfr_design3.inp') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not obvious to the user what they should and shouldn't edit.
If you're going to do this input file thing, it should only be variables that the user can customize, nothing else. It means doing the path join later in the input parsing step, but it will keep your input file clean.
saltproc/mcsfr/run_saltproc.py
Outdated
# material file with fuel composition and density | ||
mat_file = os.path.join(input_path, 'iter_mat_file') | ||
|
||
init_mat_file = os.path.join(input_path, 'mat_composition3.inp') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one deserves a description just like the others have.
saltproc/mcsfr/run_saltproc.py
Outdated
# executable path of Serpent | ||
exec_path = '/projects/sciteam/bahg/serpent30/src/sss2' | ||
|
||
# Number of cores and nodes to use in cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the cluster
|
||
Copyright (c) 2015--, Ariel Rokem, The University of Washington |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should keep a license file called LICENSE_shablona which credits ariel for the shablona backbone of this work. THat's the point of an attirbution license. You don't just delete it.
saltproc/version.py
Outdated
""" | ||
|
||
NAME = "saltproc" | ||
MAINTAINER = "Ariel Rokem" | ||
MAINTAINER_EMAIL = "arokem@gmail.com" | ||
MAINTAINER = "Jin Whan Bae" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think either Andrei or arfc is the maintainer.
saltproc/version.py
Outdated
MAINTAINER = "Ariel Rokem" | ||
MAINTAINER_EMAIL = "arokem@gmail.com" | ||
MAINTAINER = "Jin Whan Bae" | ||
MAINTAINER_EMAIL = "jbae11@illinois.edu" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the long term, arfc@googlegroups.com is best.
saltproc/version.py
Outdated
MAINTAINER = "Ariel Rokem" | ||
MAINTAINER_EMAIL = "arokem@gmail.com" | ||
MAINTAINER = "Jin Whan Bae" | ||
MAINTAINER_EMAIL = "jbae11@illinois.edu" | ||
DESCRIPTION = description |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need description
AUTHOR = "Ariel Rokem" | ||
AUTHOR_EMAIL = "arokem@gmail.com" | ||
LICENSE = "BSD-3" | ||
AUTHOR = "Andrei Rykhlevskii" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can have a list of people.
#31 is addressed in the last 4 commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge before start refactoring
This PR includes addition of the following capabilities:
run_saltproc.py
) to drive saltproc run (solves Look for file paths in the environment and input arguments #27, Look for serpent executible in the environment and input arguments #28 )