Updates in reactions and heat_capacity modules
yoelcortes opened this issue · 8 comments
The reactions and heat_capacity modules have finally been introduced. It comes with several new data lookup functions, optimizations, and a few changes.
We now have lookup methods for energies at each phase: Hfs, Hfl, Hfg, S0s, S0l, S0g. The lookup methods for solids search only the CRC data. The column names for the data tables have been changed to fit this convention (e.g. Sf(g) -> S0g, Hf(g) -> Hfg, Sfc -> S0s). These changes are also reflected in the commented sections of elements.py that use CRC_standard_data.
A couple of tests for the heat_capacity module are not passing, but they should be! The results for the chemical property functions are very close. All equations and coefficients are the same, so the problem may possibly come from fluids.assert_close. The tests use values from literature to compare and the small difference leads to these assertion errors. If you'd like, I can increase the tolerance of these methods to make sure the assertions pass.
Hi Yoel,
The issue in heat_capacity.py is that I had upgraded the ideal gas constant in fluids to the latest one. Mercifully, it will never be changed again; it is now a "Defined" constant. I merged the change and a few other changes to heat_capacity.py I had locally.
reactions.py is looking amazing! You have given amazing vare. It looks like I need to document and better test balance_stoichiometry now.
Fantastic work. The end (of the beginning) is in sight. I am most of the way through the transport properties and have work started on the other files. Do you want to talk about combustion now?
Never say never right? Just kidding, the constant is universal, so you're right on the never here jaja. I've seen your comments on scipy and the constant, so I'm guessing it had something to do with scipy.
Thanks for the nice comments. I'll get back to you over the weekend to talk about combustion. I'm sure we could establish something even better enhancements than what has been made in thermosteam. I first just need to get some good progress on a biorefinery for the production of olefins from biologically produced medium/long-chain fatty acids.
So thermosteam's Reaction object has a correct_atomic_balance method. Its not well documented yet nor is it 100% completed, but you might get some ideas from it:
from thermosteam import Chemicals, settings
>>> from thermosteam.reaction import Reaction
>>> settings.set_thermo(Chemicals(['Water', 'CO2', 'O2', 'CH4']))
>>> rxn = Reaction('CH4 -> Water + CO2 + O2', reactant='CH4', X=1)
>>> # Speed would be much faster with numba, but I'm not concerned with speed here
>>> %timeit rxn.correct_atomic_balance(constants=['CH4', 'CO2'])
77.3 µs ± 998 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)
>>> rxn
Reaction (by mol):
stoichiometry reactant X[%]
2 O2 + CH4 -> 2 Water + CO2 CH4 100.00It basically works with arrays and numpy.linalg.solve, so you probably won't find this too useful
https://github.com/BioSTEAMDevelopmentGroup/thermosteam/blob/master/thermosteam/reaction/_reaction.py
EDIT: I just saw your balance_stoichiometry, I think your code is much nicer! I'll see if I can use either that null_space or your balance stoichiometry function. I'm still having some bugs with my function...
Hi Yoel,
A couple of notes.
Notes on heat capacity.py
The spacing of the documentation for most of the functions got messed up. I needed to update the doctest results too, which were all wrong, for the gas constant reason.
I am struggling with the Zabransky classes; you took all the old stuff out. I guess it was for numba? I tried a little getting the class to work with numba, haven't had much success yet. Can you please explain what's going on with zabransky?
Notes on reaction:
ATCT is a way more reliable source than the API TDB; also, the CRC handbook is more reliable, but ATCT is most reliable. I changed the default in the one place this was not the case.
Also, I see you fixed something in reactions.py, but I'm not sure what. Good discipline is to add a test for whatever was broken, whenever you fix something. Does this apply here?
Hi Caleb,
I just saw this, sorry I missed it.
The previous implementation for Zabransky classes is not optimized, nor will it work well with numba (many things to change!) I think its best to discuss this before moving forward. Also, I thought this could potentially be implemented in thermo, so I removed it (as opposed to commenting it out). I will use comments next time to be more transparent.
Thanks for adjusting the heat of reaction sources. I added "CRC" to the S0l_methods and S0s_methods tuples. I'll try to separate out my commits next time and give more detailed names for them.
When you say spacing, are you referring to the number of lines between functions? I usually leave 1 line in between functions and 2 lines in between classes, and sparingly use lines in code blocks (I picked this up with pep8). Let me know if there is a convention you'd like to follow. Ahh, documentation spacing, how did it get messed up? Did I change something?
Do you think we could have a call sometime tomorrow evening to discuss numba, classes, and standards for documenting changes? I'd like to get this right and make sure we are on the same page.
Thanks!
Hi Yoel,
Talking sounds good, but not tonight. Don't worry about doing the "wrong" thing. I gave you contributor privileges because I wanted you to feel empowered to make changes you thought were necessary or improvements. Right now the "bus" factor of all my work is 1; I hope to make chemicals be 2.
You don't seem like someone who will feel that their code is "them" and that is has to be their way. I can revert changes I disagree with, hopefully after explaining why. I will trust you have the project's best interests at heart when you make a change, and I expect you to argue back if you disagree with something I do.
Dropping Python 2 support for chemicals was an OK decision for me, but I would like to keep supporting 3.5 and up for the initial release and a little while. Libraries don't exist in a vacuum - people use them, and often times an upgrade to their Python version is a massive undertaking. I spend 3.5 months upgrading something hundreds of thousands of lines from Python 2.6 to 3.7; I finished less than a year ago. It sucked. It really sucked. New features are really tempting and there are LOTS of them; but most users of most projects are happy with how those projects are working, and just don't want the features to break.
I am interested in Numba now, but other than maintaining support I don't know how likely I am to use it in the future. I am happy to work on the implementation of the interface to numba myself - and if you could take the role of a user, and tell me where the performance/return type/etc is inadequate, that would great with me. I realize making everything compatible is not a reasonable goal, given numba's current feature list.
I am pretty far along. I pushed some more changes, I think I have a solution to changing the return type of some of the functions to return numpy arrays.
I guess classes are really just not part of numba's feature set, so I suggest the addition of functions to calculate the Cp, S, H for the Zabransky methods.
The documentation spacing thing is likely a line ending problem - git doesn't handle line endings as well as it should. I use UNIX style, but I have never had to pay much attention to it before. I would have had a much easier time fixing if it I realized that was what the issue was. Don't worry about it :)
I try to loosely follow pep8 but mostly I think style guides just lead to angry people arguing over things that don't matter very much.
I added a changes.rst file; let's document any changes there when we consider something "good".
Cheers,
Caleb
Sounds great. I view this project as a fun, positive learning experience and hope I can bring new things to the table. I'm trying to learn all I can to be a good BDFL for BioSTEAM. Thanks for having me here!
EDIT: I'd like to try out an object oriented implementation of Zabransky since I feel like integrals would be difficult to implement on a functional level.
I've implemented jit classes before, and I'm sure I can make some jit classes for Zabranky. Although jit classes have many limitations, having the whole package support numba would be an exciting feature. On the downside, not sure if you've heard, but jitted classes are slower than python classes outside njit/jit functions (e.g. getting and setting attributes). So there is a chance things can slow down a little. I'll go ahead and take care of adding in the jitted Zabransky classes as well as the safety.py module.
Feel free to make a new issue or rst file to note down any details for python 3.5 compatibility (especially the ones I miss). And thanks for creating the changes.rst file, its a good idea.
About pep8, yeah, people code-grammar police at times jaja. I don't care much as long as things are legible. And I know I have a particular coding style that doesn't necessarily align with pep8.
Thanks!
I believe this is taken care of by now. I really appreciate the work on safety.py, it's a big file.