8
One problem that immediately jumps out at me is the loop-carried dependency through addps, which has a latency of either 3 or 4 (depending on the processor) while there are not nearly enough instructions there to fill all that time, so it's lost throughput. The typical solution is unrolling and using multiple accumulators. There's too much stuff there for me ...
8
Just reviewing normalizeFeatures.
Instead of a comment explaining what the function does, write a docstring. (Docstrings are available from the interactive interpreter via the help function.)
The function operates on the global variable X. This makes the function inflexible (you can't use it for anything other than modifying the particular variable X), and ...
7
Vectorized approach:
res = sum(abs(x(:).'.^(2:numel(x)+1)))
Read more about vectorization techniques here.
Thus, your function would look like this:
function res = a3_funct(x)
res = sum(abs(x(:).'.^(2:numel(x)+1)));
return
7
Vectorization
If you look at all the calls to mean, you see that what you are calculating in each loop is a "Cumulative moving average", but in the oposite direction of what one usually would do.
A cumulative moving average can be calculated quite simply using the cumulative sum, cumsum, and dividing by the number of elements. Since you're starting with ...
7
For each group in your data table, your code computes the coefficient b1 from a linear regression y = b0 + b1*x + epsilon, and you want to run this regression and obtain b1 for observations 1-12, 2-13, 3-14, ..., 989-1000. Right now you are separately calling lm for each data subset, which is a non-vectorized approach.
Vectorization of prediction models ...
7
Regarding the vectorized code:
permute can be considered a "Zero-cost operation".
Logical operations are performed fastest using bsxfun.
Arithmetic operations are fastest using bsxfun.
Therefore, it's close to impossible to improve the performance of the vectorized code you've posted.
Now, the question is: Is it correct?
It's hard to tell without knowing ...
7
You can use np.roll to compute the centered differences as a vectorised operation rather than in a for loop:
res = (np.roll(y, -1) - np.roll(y, 1)) / (np.roll(x, -1) - np.roll(x, 1))
You then still need to account for incorrect values at the boundaries.
Also note that you have a bug at the line
size = len(y0)
It should be len(y). But you don't need it ...
6
There may exist a way to make this particular loop run faster. In fact, I'm sure there probably is... however, it's kind of a futile effort.
There's a lot of time also being wasted elsewhere where we're creating these original three arrays we're passing in here. We need to take a far more OOP approach to this problem.
I don't know what this code actually ...
6
This is a typical use case for numpy.where:
cPrime = np.where(C, C * 30 + 1, C)
This is about twice as fast as (30 * C + 1) * (C != 0) and generalizes more easily to other conditions.
6
Since this is homework, I won't give you the exact answer, but here are some hints.
== is overloaded in numpy to return an array when you pass in an array. So you can do things like this:
>>> numpy.arange(5) == 3
array([False, False, False, True, False], dtype=bool)
>>> (numpy.arange(5) == 3) == False
array([ True, True, True, False, ...
6
The following function works without for loops or any function of the *apply family. Furthermore, it does not require additional packages but makes use of base functions only. See the code comments for further details.
significant_drydowns <- function(rain, threshold = 10, k = 48) {
# all values except the first one
rain_tail <- rain[-1L]
# ...
6
A few notes:
I'm confused about the design of your function. If you are only processing column vectors, why not just process a row vector instead? That's what I would expect when calling this function.
my_function([1:2:20]') % how I have to call your function; convoluted and unexpected
my_function(1:2:20) % how I expect to call your function; cleaner
...
6
To answer your question, you need to add a new dimension to the ndarray:
vecs /= mags[..., np.newaxis]
However
uniformly distributed unit vectors around the unit circle
No it's not, at least not in \$\theta\$. You're generating uniformly distributed points on the unit n-sphere and modifying it to the unit circle; effectively reducing it to an angle. ...
6
Maybe there's an elegant way for me to instead not delete the e column and still remove duplicates efficiently without considering the e values?
Your set-based approach almost works, and should be more efficient than the nested loops. Try storing only the first 4 items in the set, as opposed to the entire row:
def unique_by_first_n(n, coll):
seen = set(...
5
If you just want to do this for the unit circle, you can use numpy.random.uniform to choose the angle \$\theta\$ defining the unit vector.
If you want to deal with the more general case of the \$n\$-sphere, there is a method mentioned on Wikipedia.
To generate uniformly distributed random points on the \$(n − 1)\$-sphere (i.e., the surface of the \$n\$-...
5
There's a lot going on in this script, but I've tried to cover all of the important parts.
Style and conventions:
I think your coding style is very good.
You're using nice a descriptive variable names - Good!
Your variable names are consitent (camelCase) - Good!
(Except pixels_per_mm)
You're using spaces - Good!
(Most of the places, [pathstr,name,ext]=...
5
Compute legendre(n) once per loop iteration instead of twice.
Use np.hypot instead of np.sqrt(x**2 + z**2).
Don't use phivec, just pass the arrays X and Y directly to phi, which is already almost vectorized. The only change that's needed is to use np.minimum and np.maximum instead of min and max respectively.
Revised code:
def phi(x, z, nlimit=35):
r = ...
5
You can vectorize the calculation
def compute_numerical_derivative3(func, x, method='custom'):
y = func(x)
if method == 'custom':
res = np.zeros_like(y) # 'd' for double
# centered differences
x_dif = x[2:] - x[:-2]
y_dif = y[2:] - y[:-2]
res[1:-1] = y_dif / x_dif
res[0] = (y[1] - y[0]) / (x[1] - x[0])
...
5
The code seems to get more and more questionable as we read downward. Starting at the bottom:
inline auto generate_px8n_palette()
{
std::vector<px32n> result;
result.reserve(256);
for (auto i = 0; i < 256; ++i)
result.emplace_back(reinterpret_cast<px8n&>(i));
return result;
}
Writing auto instead of explicitly std:...
4
You can avoid the nested loops using numpy.meshgrid to build a table of entries in x and y, and numpy.vectorize to apply a function to all entries in the table:
def tabulate(x, y, f):
"""Return a table of f(x, y)."""
return np.vectorize(f)(*np.meshgrid(x, y, sparse=True))
For example:
>>> import operator
>>> tabulate(np.arange(1, ...
4
A few notes: (I implemented this in Octave, so there may be differences between my code and a proper Matlab implementation, but there shouldn't be).
First we care about only unique dates, right? So lets grab
indices for their groupings. I'm assuming this original data is
stored in the matrix A.
[X, y, z] = unique(A(:, 4))
Now z is the only value I used ...
4
I see two major way to simplify and speed this up. First, rather than copying a to c at every step of the loop, you can define c to include a from the beginning. Second, you can find all the nonzero, non-NaN values in a vectorized manner at the very beginning This reduces the time for me be about 1/2 for a large (~10000 row) random data set.
function c = ...
4
why not load 4 packed __m128 and then store 32 Pixels at once
I think that's 16 pixels, but it's a good plan. The pack instructions were used inefficiently (in the linked question that was not really an issue) and that would be improved by processing more elements simultaneously. For example, something like this:
#define SSE_STRIDE 16
void ConvertToUint8(...
4
You can firstly change by difference != 0 and then use na.locf to replace NAs by last available value recursively.
minimum_new <- function(data) {
answer <- rep(NA, length(data))
difference <- c(0, diff(data, lag = 1, differences = 1)) / 2
answer[1] <- data[1]
answer[difference != 0] <- data[difference != 0] - difference[difference != ...
4
If you were to add a cat("hello") at the top of your all_in function, you would find that your function is called 25 times, once for each combination (pair) of pathways. So yes, despite having used Vectorized, it is still essentially a big old loop you have under the hood... Here is how I would write a vectorized function so the heavy-lifting function (%in%, ...
4
This is easy using numpy.roll, for example:
zx = np.roll(x, 1) * (np.roll(x, 2) + np.roll(x, -1)) - x
3
Warning: I don't have Matlab installed and haven't done much with it, so my syntax may be off.
% for each i, output(i) is the maximum between output(i+1) and mean(output(i+1:end))
for i = 1:a-1
output(i,1)= max(mean(dens(i+1:a,1)),dens(i+1,1));
end
output(a,1) = dens(a,1);
If I understand that correctly, the % line is a comment. Then you have a for ...
3
This code looks good to me: the docstring is clear and the implementation is simple and efficient. So I have only a few minor points.
The code doesn't fit in 80 columns, meaning that we have to scroll it horizontally to read it here on Code Review.
The docstring contains an example. If it were formatted like this:
>>> starts = np.array([1, 3, 4, 6]...
3
Re-formulating the problem into a matrix multiplication one speeds up the algorithm very much.
Algebra:
Create a matrix G of n_groups x n_users such that every column is a group mask, that is, all users (columns) that belong to that group (row) are flagged are 1 and 0 otherwise. If Y is the matrix of interactions, then the interactions between every pair ...
3
Besides the hints in comments, I would like to add the following:
Replace the abbreviated variables by useful names. This will increase the performance of your code reviewer.
Try to keep all your comments in English (see 1.).
Preallocate variables if you really want to write their value inside a loop. Even in C++ you would not write Ta(i,t)=Tinitial;.
Do ...
Only top voted, non community-wiki answers of a minimum length are eligible
