Here is my code simulating liquid in a pipe cooling under different conditions. How did I do approaching this in an object orientated way? Is there any way I can improve this code?

import numpy as np
import scipy
from scipy.optimize import minimize
from matplotlib import pyplot as plt

class pipeSimulator:
    R_w = 0.654#  W/mk water heat loss ocefficient
    # R_w = 0.028# W/mk air heat loss ocefficient
    R_f = 0.042  #  W/mk pipe heat loss coefficient
    inner_radius = 0.2  #  m pipe radius
    T_0 = 50  #  C initial temperature
    T_a = 20  #  C ambiant temperature
    dx = 0.01  #  m differential distance
    Length = 25  # m length of pipe

    def heat_balance(self,T_1,T_2):
       heat_in = (T_1-T_2) * scipy.pi * self.inner_radius**2 * self.R_w
       heat_out = ( (T_1 + T_2) / 2 - self.T_a ) * 2 * self.inner_radius * scipy.pi * self.dx * self.R_f 
       return abs(heat_in-heat_out)

    def simulation(self):
       T = [self.T_0]
       X = [0]
       for x in np.arange(self.dx,self.Length,self.dx):   
         fun = lambda s: self.heat_balance(T[-1],s)

trial = pipeSimulator()
trial.Length = 30
diameters = [0.02,.2,1,3]
R_values = [100,400]
T_a_values = [20]
cols = ['red', 'blue', 'black', 'purple', 'brown']
mylines = ['-','--','-.',':']
labs = []
for rval in R_values:
    trial.R_f = 1.0/rval
    for dval in diameters:
         trial.inner_radius = dval/2.0
         for tval in T_a_values:
              trial.T_a = tval
              ans = trial.simulation()
              labs.append(str(rval)+' R-value (EU), '+str(dval)+' meter diameter, '+str(tval)+" degree C ambient")

plt.xlabel('Distance (m)',fontsize=20)
plt.ylabel('Average Temperature (C)',fontsize=20)

Immediately you have bad names of constants. R_w, R_f, T_0? These are meaningless. If they correspond to technical terms, then make a note of that in a single comment, ideally with a link to somewhere online that they could be roughly explained. But the best response would be actual words:

class PipeSimulator:
    INNER_RADIUS = 0.2  # m
    LENGTH = 25  # m

    # Coefficients (in W/mk)
    WATER_HEAT_LOSS = 0.654
    # AIR_HEAT_LOSS = 0.028
    PIPE_HEAT_LOSS = 0.042

    # Temperature C

Now, you'll notice I changed the names but also the naming style to make it clearer that they're constants. It's important to use this convention. It's useful to note that these are coefficients and what unit they're in (I assume that's what W/mk is) but instead of noting it individually put a single comment header under which they all fall. For units on single constants, simply leaving the unit notation at the end is sufficient.

However, I question whether these should all be constants. Wont you have pipes of differing lengths and inner radius? I'm not familiar with the physics here, but will the coefficients change contextually? It seems like at least some of these values should be initiated as instance attributes with __init__. Indeed, some of them are set by you again later. You'd save some lines by setting up __init__.

To give a simple demo of how that would look, I'll just use inner radius and length:

class PipeSimulator:

    def __init__(self, inner_radius=0.2, length=25):
        self.inner_radius = inner_radius
        self.length = length

pipe1 = PipeSimulator(0.2, 25)
pipe2 = PipeSimulator(0.6, 40)
pipe3 = PipeSimulator()

This way, you can pass different parameters when creating PipeSimulator to get different attributes on your object. I'm also using default parameters, in case you'd want the class to fall back on a default radius on length for when none are supplied (like in pipe3). If this is completely unfamiliar to you, leave a comment and I can explain how it works.

Note that __init__ can have other lines of code too, so if some attributes need to be calculated it's perfectly fine to have them determined in there rather than calculating externally to then assign to your PipeSimulator object.

heat_balance has a complex equation to determine the result. I'd suggest putting in a docstring where you make reference to what this equation is and where it could be found. It would be more readable too if you split the long lines up, calculating only a bit at a time instead of everything thrown together in an unreadable manner. If you wanted to be really neat, heat_in and heat_out could each have their own function that just get called in heat_balance.

The same applies to simulation except that I don't think it's a formula so much as your methods and intentions are not the clearest. What are T and X for? If you gave them better names it would be much easier to follow what they mean. Add a docstring too, explain what is simulated here. Though your question title explains it, nothing in the code itself does.


First suggestion: Look at the answers to your last post. I'll repeat some of the points regardless.

In general please look at PEP8 to make the code more aligned with Python standards; in particular

  • keep the indentation consistent, best would be exactly four spaces, and
  • classes should be named in camel case, i.e. PipeSimulator, as was already mentioned.
  • Also, give all things a bit more spacing, otherwise lines are really hard to decipher with all the operators added. Meaning at least a space after commas.
  • return is not a function, so the argument shouldn't be wrapped in parenthesis.

You already got a good response for the class, so I'll continue with methods and plotting.

I'd strongly suggest to move the plotting into a separate function or method as well. There are also a few niceties to reduce the number of lines and nesting by using some functional and iterator-based tools, in particular the itertools library.

I think the following looks a little bit cleaner:

from itertools import cycle, izip, product

import numpy as np
import scipy
from scipy.optimize import minimize
from matplotlib import pyplot as plt

class PipeSimulator:
    R_w = 0.654  #  W/mk water heat loss coefficient
    # R_w = 0.028  #  W/mk air heat loss coefficient
    R_f = 0.042  #  W/mk pipe heat loss coefficient
    inner_radius = 0.2  #  m pipe radius
    T_0 = 50  #  C initial temperature
    T_a = 20  #  C ambient temperature
    dx = 0.01  #  m differential distance

    def __init__(self, length=25):
        self.length = length

    def heat_balance(self, T_1, T_2):
        heat_in = (T_1 - T_2) * scipy.pi * self.inner_radius**2 * self.R_w
        heat_out = ((T_1 + T_2) / 2 - self.T_a) * 2 * self.inner_radius * scipy.pi * self.dx * self.R_f
        return abs(heat_in - heat_out)

    def simulation(self):
        T = [self.T_0]
        fun = lambda s: self.heat_balance(T[-1], s)
        xs = np.arange(self.dx, self.length, self.dx)

        for x in xs:
            T.append(float(minimize(fun, T[-1] - self.dx, method="SLSQP")['x']))

        return (T, [0] + list(xs))

    def plot(self):
        diameters = [0.02, 0.2, 1, 3]
        R_values = [100, 400]
        T_a_values = [20]

        parameters = product(R_values, diameters, T_a_values)

        cols = ['red', 'blue', 'black', 'purple', 'brown']
        mylines = ['-', '--', '-.', ':']
        nlines = len(mylines)
        fontsize = 20

        labs = []
        for i, ((rval, dval, tval), col, line) in enumerate(izip(parameters, cycle(cols), cycle(mylines))):
            self.R_f = 1.0 / rval
            self.inner_radius = dval / 2.0
            self.T_a = tval
            (x, y) = self.simulation()
            plt.plot(y, x, c=col, ls=line, lw=2 * (i % nlines + 1))
            labs.append('{} R-value (EU), {} meter diameter, {} degree C ambient'.format(rval, dval, tval))

        plt.legend(fontsize=fontsize, title='', labels=labs, prop={'size': 12}, loc=3)
        plt.xlabel('Distance (m)', fontsize=fontsize)
        plt.ylabel('Average Temperature (C)', fontsize=fontsize)

if __name__ == "__main__":
    simulator = PipeSimulator(length=30)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.