6
\$\begingroup\$

I've written a tool in Python to help me with a specific problem I have with electronic circuit design. Here are the motivations:

  1. It is common in circuit design to need to choose two resistor values to form a voltage divider. Any number of combinations of resistors can satisfy the divider. And there are a lot of available resistor values to choose from. It is not uncommon for an engineer to just pick a resistor value out of thin air and solve for the other.

  2. It is very desirable to reduce the number of line items in the BOM for a circuit board. In other words, the more resistor values I can share elsewhere in the same circuit, the better.

Prior to running this program, I have created lists of candidate resistor pairs and their combined cost in separate files. The contents of these JSON-encoded files look like this:

[[RA1, RA2, priceA],[RB1,RB2,priceB],[RC1,RC2,priceC],...]

Every pair of resistors in the same file will satisfy some specified voltage divider ratio. I have as many files as I have unique voltage dividers in the circuit.

My program then takes the multiple JSON files as input arguments, generates every permutation of resistor combinations, and tries to find which permutation produces the least number of unique resistor values. I also have it only show me the outputs with the lowest combined resistor price (many resistors are the same cost, so this helps pare down the results).

I am very new to Python and come from a long history of coding in C. I want to learn how to make this code more Pythonic. In particular, I'm convinced the embedded for-loops in the middle of the code can be replaced with something much more efficient.

resistor_bom_reducer.py:

"""
  python3 resistor_bom_reducer.py file1.json file2.json file3.json ... fileN.json
""" 

import json
import sys
import itertools

pairlists = []
totalperms = 1

# Read in JSON data from input arguements.
# pairlists is a list of lists representing every pair of resistors and their combined cost
for filename in sys.argv[1:]:
    with open(filename) as json_data:
        lst = json.load(json_data)
        totalperms *= len(lst)
        pairlists.append(lst)

print("Calculating permutations: %d" % totalperms)
perms = list(itertools.product(*pairlists))
print("Done")

# Loop through every permutation of resistors.
# combo is a list of flattened lists of every resistor combination
combo = [[0 for rows in range(len(pairlists)*2)] for cols in range(len(perms))]
price = [0 for x in range(len(perms))]
count = 0
for i in range(0,len(perms)):
    for j in range(0,len(pairlists)):
        combo[i][2*j] = perms[i][j][0]
        combo[i][2*j+1] = perms[i][j][1]
        price[i] += perms[i][j][2]
        count += 1
        if ( count % 1000000 == 0 ):
            print("%d of %d" % (count,totalperms))

print("")

# Find the combination with the least number of unique resistor values
minlen = 999
for s in combo:
    if ( len(set(s)) < len(pairlists)*2 ):
        if ( len(set(s)) < minlen ):
            minlen = len(set(s))

# Find the lowest combination price
minprice = 999.0
for s,p in zip(combo,price):
    if ( len(set(s)) == minlen ):
        if ( p < minprice ):
            minprice = p

# Print only combinations of the lowest price and least number of unique resistor values
for s,p in zip(combo,price):
    if ( len(set(s)) == minlen ):
        if ( p == minprice ):
            print(s,p)
\$\endgroup\$
  • \$\begingroup\$ (i) The code insists on having a minimum set of resistor values, and only compares price among the minimum sets (if there are more than one). But this seems unrealistic — surely you'd prefer to have a larger set of resistor values if it was cheaper overall. (ii) The code seems to assume that each divider has only two resistors. But resistors can be combined in series to get the sum their resistance. Is this not an option when building voltage dividers? \$\endgroup\$ – Gareth Rees Feb 3 '18 at 13:19
2
\$\begingroup\$
    with open(filename) as json_data:

Good!


        lst = json.load(json_data)
        totalperms *= len(lst)
        pairlists.append(lst)

lst isn't a great name, but I think it's forgiveable with such a tight scope. Maybe worth sanity-checking that the structure is what you expect?


print("Calculating permutations: %d" % totalperms)
perms = list(itertools.product(*pairlists))

Why bother with the code to calculate totalperms if you can just extract it as len(perms)?

Although, on the flip side, why coerce perms to a list and force it all into memory?


# combo is a list of flattened lists of every resistor combination
combo = [[0 for rows in range(len(pairlists)*2)] for cols in range(len(perms))]

I'm not sure what the point of this is. Perhaps the comment should explain why we need it.


for i in range(0,len(perms)):

You're right to think there's a more Pythonic way of doing this:

    for i, perm in enumerate(perms):

This doesn't require perms to be a list, because it removes the need to index into it.


    for j in range(0,len(pairlists)):
        combo[i][2*j] = perms[i][j][0]
        combo[i][2*j+1] = perms[i][j][1]
        price[i] += perms[i][j][2]

This really isn't doing much. Why not inline the calculation of the best option?


        count += 1
        if ( count % 1000000 == 0 ):
            print("%d of %d" % (count,totalperms))

That's buggy: it should be at one less level of indentation.


# Find the combination with the least number of unique resistor values

This is a pet peeve of mine: unique means that there is only one in existence. Distinct expresses precisely and unambiguously what you mean here.


minlen = 999
for s in combo:
    if ( len(set(s)) < len(pairlists)*2 ):
        if ( len(set(s)) < minlen ):
            minlen = len(set(s))

It's not standard Python style to use brackets around the condition. The conditions can be merged into one as

    if len(set(s)) < min(len(pairlists)*2, minlen):
        minlen = len(set(s))

but then that points to just initialising minlen to len(pairlists)*2 instead of the magic number 999.


Finally, this doesn't strike me as likely to be an efficient approach.

I would think it's likely to be more efficient (although still not very) to build a set of the distinct values and then look at subsets in order of increasing size, maybe guided by frequencies of the values.

The best option is probably to try to work out an encoding to a standard problem and use an optimised solver. The obvious one is SAT: you've basically got a Boolean formula already: each resistor value corresponds to a variable, and each file is an OR of a bunch of two-variable ANDs. Then you just need to add a count of the number of variables which are TRUE and require it to be less than a threshold, and search for the critical value of the threshold. For the prices you can use the same SAT formulation and loop:

while sat_instance.solve():
    yield sat_instance.solution()
    add a rule to sat_instance to exclude the solution
\$\endgroup\$

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.