Cleanup reprocessing-related classes and code #160
Conversation
- make internal variables more descriptive - clean up loop variables - avoid calling full dictionary path where possible
- expand function names - iso,el -> nuc - rewrite docstrings - add thru_flow return parameter to process_materials - rem_elements -> process_materials
yardasol
force-pushed
the
cleanup-reprocessing
branch
2 times, most recently
from
a24d3a1
to
f155719
Compare
yardasol
force-pushed
the
cleanup-reprocessing
branch
from
f9c3a2f
to
83e4acf
Compare
We do this so that we can retain the other attributes from inflow
There was a problem hiding this comment.
This PR primarily changes names and function doc strings for clarity. Some of the name changes are neutral (w.r.t to clarity). For example "read_materials_from_input" --> "get_materials" is neutral, to me, because "get_materials" is less specific but much cleaner.
As long as tests still pass, I'm happy to merge.
saltproc/process.py
Outdated
| # print("Xe concentration in inflow before % f g" % inflow['Xe136']) | ||
| # print("Current time %f" % (t)) |
There was a problem hiding this comment.
Are these print statements erroneous?
There was a problem hiding this comment.
Hmm, they aren't erroneous, but I don't think they need to be here. i'm okay with purging them. Alternatively, I could leave them as-is for now and create a verbose command line option that would allow a user to specify that they want this extra infro printed.
There was a problem hiding this comment.
@samgdotson thoughts?
There was a problem hiding this comment.
I see your options being:
- uncomment them
- uncomment them and make an issue to add a verbosity feature
- delete them
It doesn't matter to me which option you choose, but leaving commented print statements in your code doesn't seem like good practice (I am also guilty of this).
There was a problem hiding this comment.
Agreed. I'll do option 2.
| print("Xe concentration in thruflow: %f g" % thru_flow['Xe136']) | ||
| print("Waste mass: %f g\n" % waste_stream.mass) |
There was a problem hiding this comment.
This makes me think that the above-commented print statements should be uncommented.
| @@ -21,6 +21,139 @@ | |||
| import numpy as np | |||
|
|
|||
|
|
|||
| def run(): | |||
There was a problem hiding this comment.
Did anything about this function change besides its location?
There was a problem hiding this comment.
Other than using the new function names and signatures, (and some adjustments to the print statements), no.
…reprocessing Cleanup reprocessing-related classes and code 86ed1a2
…nup-reprocessing Cleanup reprocessing-related classes and code 86ed1a2
Summary of changes
This PR addresses various, inconsistencies, unclear functionality and comments, and some inefficiencies in the functions and classes that perform reprocessing work in SaltProc.
Specifically, this PR does the following:
app.pyreprocessing()->reprocess_materials()refill()->refill_materials()read_feeds_from_input()->get_feeds()read_processes_from_input()->get_extraction_processes()read_dot()->get_extraction_process_paths()deepcopycalls where possible inreprocess_materialsprocess.pyProcess.calc_rem_efficiency()->Process.calculate_removal_efficiency()sparger.pySparger.calc_rem_efficiency()->Sparger.calculate_removal_efficiency()separator.pySeparator.calc_rem_efficiency()->Separator.calculate_removal_efficiency()Types of changes
Required for Merging
Associated Issues and PRs
None
Associated Developers
Checklist for Reviewers
Reviewers should use this link to get to the
Review Checklist before they begin their review.