Repository navigation
Conversation
- swapaxes and moveaxis build their permutation with the unsigned index type of the shape instead of the signed strides type. The values are axes, and transpose converts them to size_type anyway. - quantile builds its index list and intermediate shapes with unsigned index types, and passes the original signed axis to partition, fancy_indexing and moveaxis instead of converting the normalized one back. - call_over_leading_axis computes the number of slices as a ptrdiff_t, since it is only used in pointer arithmetic with the signed stride. - argsort_iter converts the sorted indices to the difference type of the data iterator. They come from iota over the data range, so they fit. - xindex_view::element converts the index it receives to size_type. reshape_view passes signed indices (from unravel_index) that are never negative.
The refactoring to std::apply in xtensor-stack#2876 dropped the static_cast<size_type>(args)... that the comment above it still describes. Calling an xfunction with int indices, for example t(0) on an xindex_view expression, then warned about sign conversion in the operands' access methods. Restore the cast.
The constants were double literals, so numeric_constants<float> converted them from double, which -Wfloat-conversion reports. Use the C++20 std::numbers variable templates instead. The values for float and double are bit-identical to the previous ones (pi_v / 2, 2 * inv_pi_v and so on are exact operations). numeric_constants<long double> now has full long double precision instead of double precision.
xfft divided complex expressions by sizes (N, m, n) and multiplied pi by an integer tensor, so xoperation.hpp converted size_t to float or double inside the library. Convert explicitly to the precision the expression already used, which keeps the results unchanged. N and m are powers of 2, so they are exact. The bluestein angle constant now uses numeric_constants<precision>::PI. read_header_1_0 assembled the 16-bit header length in uint16_t, which is promoted to int and narrowed back. Use uint32_t like read_header_2_0.
The tests multiplied float tensors by size_t values and divided complex results by size_t, which made the library headers warn. Use a float amplitude and convert the frequency and length once.
Contributor
Merging this PR will not alter performance1 warning
Comparing 
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Description
The library and the tests now build without
-Wconversion,-Wsign-conversionand-Wold-style-castwarnings, including-Wfloat-conversionand clang's-Wimplicit-int-float-conversion. Unique warning sites in the test build:SKIP_ON_WERRORThe
xbuilder.hppsites are not in this PR: they hid a bug, fixed in #2952. The 7 clang sites left come fromtest_xnan_functionschecks that already sit under#ifndef SKIP_ON_WERRORbecause they request lossy result types.Most fixes change a type: unsigned permutations in
swapaxes/moveaxis, unsigned index types inquantile, auint32_tnpy 1.0 header length like the 2.0 reader,numeric_constantsdefined fromstd::numbers. Each remainingstatic_casthas a comment explaining why the value fits.xfunction::operator()gets back thestatic_cast<size_type>(args)that #2876 dropped; the comment above it still described it. Intest_xfft, the test signal no longer mixes integer and float tensors.Behavior change:
numeric_constants<long double>now has fulllong doubleprecision, andnumeric_constants<T>requires a floating-pointT. Thefloatanddoublevalues are bit-identical.