9
it seems to be optimized quite well by the compiler, as it uses a lot of vectorized instructions
Well, kind of. GCC didn't use vectorization at all, and Clang used some of it but in a very strange way. Let's look at what it's doing. Basically I'm reviewing what Clang did, not so much your code.. but that then informs you about your code by proxy. I'll go ...
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 ...
8
There are some & 0xff operations that are not necessary:
(aMask & (~iMask & 0xff)), because the bits reset by & 0xff are already zero in aMask, so they never survive the "main" &.
_mm256_movemask_ps(...) & 0xff, because vmovmskps can only set the low 8 bits, the upper bits are already zero.
// inc = -1 for (itr < max) & (...
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
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.
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
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
There are a few things that can be lifted out of loops.
The three screen related variables, screen_height, screen_width, and screen_size, along with scanline_in and scanline_out, can be initialized before the outer i for loop.
In the main blit loop, the check for scanline_out_index <= screen_size can be moved to before the x loop (and when false, you can ...
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
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(...
6
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 ...
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
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
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]...
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 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])
...
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
4
I think a different approach is needed to achieve a better performance. The current approach recomputes the Jaccard similarity from scratch for each possible threshold value. However, going from one threshold to the next, only a small fraction of prediction values change as well as the intersection and the union. Therefore a lot of unnecessary computation is ...
Only top voted, non community-wiki answers of a minimum length are eligible
