Adding sparging system to saltproc

[mehmeturkmen]

This PR holds a sensitivity analysis script to seek an optimal sparging system. sensitivity.py under scripts/Sensitivity/ calculates removal efficiencies considering various design parameters for bubble generator and bubble separator, compares results to a defined reference design and illustrates results in terms of change in removal efficiencies with a change in design parameters.

In addition to above, this PR adds a sparging component into the function read_processes_from_input in app.py where removal efficiencies are read for each chemical isotope from json input file. The function calls a separate script sparging.py which defines a Sparger Class for bubble generator and a Separator Class for bubble separator, and uses efficiency equations reported by ORNL-TM-2245 and ORNL-TM-4533. The modified function updates removal efficiencies of chemical isotopes for the sparging system. If the efficiencies are entered as {} in the json input file, the code will calculate the efficiencies itself.

[pep8speaks]

Hello @mehmeturkmen! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file scripts/Sensitivity/sensitivity.py:

Line 93:80: E501 line too long (80 > 79 characters)

Comment last updated at 2021-02-23 17:53:23 UTC

[andrewryh]

@mehmeturkmen Thank you for your effort! I am glad someone adding more features to SaltProc.
Strategically-wise, maybe it worse to consider make Sparger and other components as Child class of Parent class Process?
I think that is what @katyhuff wanted initially.

[mehmeturkmen]

@katyhuff @andrewryh I have made necessary changes according to all the comments you made in the first review.

I redefined the Sparger and Separator classes as child classes from Process parent class. I think this was what was in your mind @katyhuff. As you said, it simplified the "if" statement. If this is not what you implied, let's discuss the point where I don't understand.

There is still one thing bothering me: the use of Sparging system as an option in saltproc. Existing use of saltproc allows a user to use own-defined sparger/separator efficiencies in input file. This definition can be a fixed value or an equation. With this PR, saltproc will evolve to automatically calculate sparger and separator's efficiencies for target isotopes. I think existing flexibility should be maintained. In the previous review, I suggested an empty string definition in input file to activate the saltproc for the calculation of the sparging efficiency. Maybe this time we should use none values associated with key target isotopes in input file to enable the option. Along with your suggestions, I will add an "if" statement before line 168 in app.py.

[katyhuff]

I will not have a chance to review this before Friday.
The tests are currently failing:

https://travis-ci.org/github/arfc/saltproc/builds/759722725

It will help my confidence in the review if you can make sure the tests pass (by fixing the code, not deleting the tests, please). Ideally, it would be best to also add tests to the test suite that cover the new functionality you have added.

[mehmeturkmen]

Ok. I am going to add new tests. Because of the reason I mentioned in previous comment, the test fails. I'll also fix it.

[mehmeturkmen]

With new patches, test files for sparger and separator are available. I also tested the results of latest saltproc with the results of existing one. They were totally the same.
I also maintained the existing saltproc input flexibility in this PR. So, besides numerical values and equation as input in efficiency key of any object name, now json object input file accepts string input in efficiency keywords of sparger and separator objects. To enable the sparging system in the saltproc, I defined the activation word as "self". For better ideas, I am open to discuss. The usage as below

"sparger": { "capacity": 9920000, "efficiency": "self", "mass_flowrate": 9920000, "volume": 10000000 }, "entrainment_separator": { "capacity": 9920000, "efficiency": "self", "mass_flowrate": 9920000, "volume": 11 },

[andrewryh]

Sorry for the late response.
Tests passed, the new classes and methods are very well defined.

At some point, in separate PR, please rebuild the documentation, so users would be aware about these new capabilities.
Thanks

[mehmeturkmen]

Good point @andrewryh. Any suggestion for the best place to put? I am making necessary informative changes in app,py documentation but how about overview file? I can mention about the new feature of the saltproc by opening a new section inside it like "New".

[andrewryh]

Good point @andrewryh. Any suggestion for the best place to put? I am making necessary informative changes in app,py documentation but how about overview file? I can mention about the new feature of the saltproc by opening a new section inside it like "New".

It is already up and running: https://github.com/arfc/saltproc/tree/master/doc
You just have to rebuild it and place it to gh-pages branch.
The documentation can be seen by anyone here: https://arfc.github.io/saltproc/

[katyhuff]

Thank you memo, I think this is a definite improvement!! After this is merged, please make a pr to the gh-pages branch that rebuilds the docs.