current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

Aim: Write a function that looks up empirical parameters A, B and C for a solar radiation model. Each parameter has a value for a certain interval of time, represented as the day of the year. For now, I basically translate the four-column (day_of_year-interval, A, B, C) table that I have on paper into the following look up. The function works, but I have a feeling that this is not the most Pythonic way to do it.

Current implementation:

import numpy as np

def lookup_DNI_parameters(day_of_year):
    """Sets parameters A, B, and C for the calculation of direct normal irradiance
    based on day of year-intervals.

    A is in W/m^2
    B and C are dimensionless
    """
    A = np.empty_like(day_of_year)
    B = np.empty_like(day_of_year)
    C = np.empty_like(day_of_year)

    # generate boolean indices for each day_of_year-interval
    interval_1 = np.logical_and(day_of_year >=1, day_of_year < 32)
    interval_2 = np.logical_and(day_of_year >= 32, day_of_year < 60)
    interval_3 = np.logical_and(day_of_year >= 60, day_of_year < 91)
    interval_4 = np.logical_and(day_of_year >= 91, day_of_year < 121)
    interval_5 = np.logical_and(day_of_year >= 121, day_of_year < 152)
    interval_6 = np.logical_and(day_of_year >= 152, day_of_year < 182)
    interval_7 = np.logical_and(day_of_year >= 182, day_of_year < 213)
    interval_8 = np.logical_and(day_of_year >= 213, day_of_year < 244)
    interval_9 = np.logical_and(day_of_year >= 244, day_of_year < 274)
    interval_10 = np.logical_and(day_of_year >= 274, day_of_year < 305)
    interval_11 = np.logical_and(day_of_year >= 305, day_of_year < 335)
    interval_12 = day_of_year >= 335

    # set parameter values based on the interval
    A[interval_1] = 1230.0
    A[interval_2] = 1215.0
    A[interval_3] = 1186.0
    A[interval_4] = 1136.0
    A[interval_5] = 1104.0
    A[interval_6] = 1088.0
    A[interval_7] = 1085.0
    A[interval_8] = 1107.0
    A[interval_9] = 1152.0
    A[interval_10] = 1193.0
    A[interval_11] = 1221.0
    A[interval_12] = 1234.0

    B[interval_1] = 0.142
    B[interval_2] = 0.144
    B[interval_3] = 0.156
    B[interval_4] = 0.180
    B[interval_5] = 0.196
    B[interval_6] = 0.205
    B[interval_7] = 0.207
    B[interval_8] = 0.201
    B[interval_9] = 0.177
    B[interval_10] = 0.160
    B[interval_11] = 0.149
    B[interval_12] = 0.142

    C[interval_1] = 0.058
    C[interval_2] = 0.060
    C[interval_3] = 0.071
    C[interval_4] = 0.097
    C[interval_5] = 0.121
    C[interval_6] = 0.134
    C[interval_7] = 0.136
    C[interval_8] = 0.122
    C[interval_9] = 0.092
    C[interval_10] = 0.073
    C[interval_11] = 0.063
    C[interval_12] = 0.057

    return A, B, C

# test an array of a few sample days for correctness of results
days = np.array([21., 80., 141., 202., 264., 325.])
A, B, C = lookup_DNI_parameters(days)
print A # [1230, 1186, 1104, 1085, 1152, 1221]
print B # [0.142, 0.156, 0.196, 0.207, 0.177, 0.149]
print C # [0.058, 0.060, 0.071, 0.121, 0.136, 0.092, 0.063]
share|improve this question
1  
Welcome to Code Review! Is this example code? It is always best to post your real working code to get the best out of a code review on this site. If this is example code, there is a chance your question could be closed as off-topic. –  Phrancis Dec 10 '14 at 17:12
    
Hi and thanks for the welcome. I originally posted the question to stackoverflow and got asked to post it here because the code in principle works, so sorry if it is inapproriate here. It is kind of example / prototype code because I honestly gave up on the real code for now (i.e. it doesn't work at the moment) due to its messiness. The real code really doesn't do (/isn't supposed to do) anything else, apart from a few more parameters than A and B and similar idxs though. –  Fred S Dec 10 '14 at 17:16
    
I see. I'll leave it up for Python reviewers to decide whether this code is reviewable. –  Phrancis Dec 10 '14 at 17:23
    
Please see Why is hypothetical example code off-topic for Code Review?. Generic identifiers like A and idx_2 and magic numbers like 22 make it hard for us to figure out what you are really trying to accomplish. I think you'll get better reviews if you show us your real code, so I'll put this question on hold to give you a chance to alter it. –  200_success♦ Dec 10 '14 at 19:44
1  
At this level of coding, numpy and MATLAB practices are similar. There is a numpy.where function, but that is only for one conditional (and its complement). –  hpaulj Dec 10 '14 at 20:26

1 Answer 1

up vote 3 down vote accepted

So you are doing a lookup over many intervals. A repeated pattern like that can be turned into a loop or vectorized

The repeated boilerplate suggests an iterative solution - looping over an array (or list) of dates and values. It doesn't save on run time, and may, or may not, be more maintainable.

def lookup_DNI_parameters1(day_of_year):
    """solution based on iterating over ranges of dates
    """
    A = np.empty_like(day_of_year)
    dates = [1, 32, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366]
    Avalues = [1230.0, 1215.0, 1186.0, 1136.0, 1104.0, 1088.0, 1085.0, 1107.0, 1152.0, 1193.0, 1221.0, 1234.0]
    for i in range(len(dates)-1):
        interval = np.logical_and(day_of_year >= dates[i], day_of_year < dates[i+1])
        A[interval] = Avalues[i]
    # similarly for B and C
    return A

The operation can also be vectorized (in MATLAB as well as numpy), by constructing a matrix that compares the whole day_of_year array against the dates intervals array(s). The [:,None] broadcasting may be novel to a MATLAB user.

def lookup_DNI_parameters2(day_of_year):
    """solution based on vectorized matching of days
    """
    dates = [1, 32, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366]
    dates1 = np.array(dates[:-1]) # interval start
    dates2 = np.array(dates[1:]) # interval stop
    Avalues = [1230.0, 1215.0, 1186.0, 1136.0, 1104.0, 1088.0, 1085.0, 1107.0, 1152.0, 1193.0, 1221.0, 1234.0]
    Avalues = np.array(Avalues)
    I = (dates1[:,None] <= day_of_year) & (day_of_year < dates2[:,None]) # boolean index array
    I = np.where(I)[0]  # convert to numeric index
    return Avalues[I]

Or this could be generalized to use a dates parameter, and a values array that has information for A, B and C:

def lookup_DNI_parameters3(day_of_year, dates, values):
    """solution based on vectorized matching of days
    """
    dates1 = np.array(dates[:-1]) # interval start
    dates2 = np.array(dates[1:]) # interval stop
    I = (dates1[:,None] <= day_of_year) & (day_of_year < dates2[:,None]) # boolean index array
    I = np.where(I)[0]  # convert to numeric index
    return values[...,I]

dates = [1, 32, 60, 91,...]
values = np.array([[1230.0, 1215.0,...], [142., 144., ...], [...]])
lookup_DNI_parameters3(day_of_years, dates, values)
share|improve this answer
    
This is great, thank you for taking the time. I also see now why what I considered to be unnecessary repetitive code may have helped you get a better idea of possible simplifications. –  Fred S Dec 12 '14 at 9:01

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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