Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changed std::pow to Utilities::pow in include/ source/ #14759

Merged
merged 1 commit into from Feb 9, 2023

Conversation

Darth-Veidim
Copy link
Contributor

as discussed in #13321.

list of changed files:

$ egrep -r 'pow.*, [2345](\.0)?\)' include/ source/
include/deal.II/optimization/line_minimization.h:   *     const double f = 100. * std::pow(x, 4) + std::pow(1. - x, 2); // Value
include/deal.II/optimization/line_minimization.h:   *     const double g = 400. * std::pow(x, 3) - 2. * (1. - x); // Gradient
include/deal.II/optimization/line_minimization.h:      std::pow(x2_shift * x3_shift, 2) * (x2_shift - x3_shift);
include/deal.II/optimization/line_minimization.h:      (r1 * std::pow(x3_shift, 2) - r2 * std::pow(x2_shift, 2)) / denom;
include/deal.II/optimization/line_minimization.h:      (r2 * std::pow(x2_shift, 3) - r1 * std::pow(x3_shift, 3)) / denom;
source/base/function_lib_cutoff.cc:                          (-2.0 * r * r / std::pow(-r * r + d * d, 2.0) * d *
source/base/function_signed_distance.cc:          std::pow(distance, 3);
source/base/function_signed_distance.cc:        val += std::pow((point[d] - center[d]) / radii[d], 2);
source/base/function_signed_distance.cc:          const double ex = (a * a - b * b) * std::pow(std::cos(t), 3) / a;
source/base/function_signed_distance.cc:          const double ey = (b * b - a * a) * std::pow(std::sin(t), 3) / b;
source/base/quadrature_lib.cc:      double eta   = (std::pow(gamma - gamma_bar, 3.0) +
source/grid/grid_generator.cc:                   0.3516 * std::pow(x, 2) + 0.2843 * std::pow(x, 3) -
source/grid/grid_generator.cc:                   0.1036 * std::pow(x, 4)); // half thickness at a position x
source/grid/grid_generator.cc:                  (x <= p) ? m / std::pow(p, 2) * (2 * p * x - std::pow(x, 2)) :
source/grid/grid_generator.cc:                             m / std::pow(1 - p, 2) *
source/grid/grid_generator.cc:                               ((1 - 2 * p) + 2 * p * x - std::pow(x, 2));
source/grid/grid_generator.cc:                                      2 * m / std::pow(p, 2) * (p - x) :
source/grid/grid_generator.cc:                                      2 * m / std::pow(1 - p, 2) * (p - x);
source/grid/grid_generator.cc:                   0.3516 * std::pow(x, 2) + 0.2843 * std::pow(x, 3) -
source/grid/grid_generator.cc:                   0.1036 * std::pow(x, 4)); // half thickness at a position x
source/grid/manifold_lib.cc:  double w     = std::sqrt(std::pow(y - std::sin(phi) * R, 2.0) +
source/grid/manifold_lib.cc:                       std::pow(x - std::cos(phi) * R, 2.0) + z * z) /
source/numerics/derivative_approximation.cc:        (std::pow(s[0][0], 3) + std::pow(s[1][1], 3) + std::pow(s[2][2], 3) +

@marcfehling
Copy link
Member

Thank you for your patch! You need to include the utilities header in function_signed_distance.cc.

[ 72%] Building CXX object source/base/CMakeFiles/obj_base_debug.dir/function_signed_distance.cc.o
/home/runner/work/dealii/dealii/source/base/function_signed_distance.cc(204): error: name followed by "::" must be a class or namespace name
          val += Utilities::pow(2, (point[d] - center[d]) / radii[d]);
                 ^

/home/runner/work/dealii/dealii/source/base/function_signed_distance.cc(248): error: name followed by "::" must be a class or namespace name
            const double ex = (a * a - b * b) * Utilities::pow(3, std::cos(t)) / a;
                                                ^

/home/runner/work/dealii/dealii/source/base/function_signed_distance.cc(249): error: name followed by "::" must be a class or namespace name
            const double ey = (b * b - a * a) * Utilities::pow(3, std::sin(t)) / b;
                                                ^

/home/runner/work/dealii/dealii/source/base/function_signed_distance.cc(79): error: name followed by "::" must be a class or namespace name
            Utilities::pow(3, distance);
            ^

compilation aborted for /home/runner/work/dealii/dealii/source/base/function_signed_distance.cc (code 2)
make[2]: *** [source/base/CMakeFiles/obj_base_debug.dir/build.make:230: source/base/CMakeFiles/obj_base_debug.dir/function_signed_distance.cc.o] Error 2
make[1]: *** [CMakeFiles/Makefile2:2607: source/base/CMakeFiles/obj_base_debug.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

@@ -418,14 +420,14 @@ namespace LineMinimization
const NumberType r1 = f2 - f1 - g1 * x2_shift;
const NumberType r2 = f3 - f1 - g1 * x3_shift;
const NumberType denom =
std::pow(x2_shift * x3_shift, 2) * (x2_shift - x3_shift);
Utilities::pow(2, x2_shift * x3_shift) * (x2_shift - x3_shift);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you switch the order of the arguments? There are supposed to be same as std::pow

@drwells
Copy link
Member

drwells commented Feb 7, 2023

I think these should be fixed_power(), as Utilities::pow() is only for integers and is intended for constexpr evaluation.

@@ -76,7 +76,7 @@ namespace Functions
const SymmetricTensor<2, dim> hess =
unit_symmetric_tensor<dim>() / distance -
symmetrize(outer_product(center_to_point, center_to_point)) /
std::pow(distance, 3);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You missed changing to fixed_power in source/base/function_signed_distance.cc

, n_cells_x_1(Utilities::pow(2, refinements) * n_subdivision_x_1)
, n_cells_x_2(Utilities::pow(2, refinements) * n_subdivision_x_2)
, n_cells_y(Utilities::pow(2, refinements) * n_subdivision_y)
, n_cells_x_0(Utilities::fixed_power<2>(refinements) * n_subdivision_x_0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to keep Utilities::pow or std::pow here.

This statement should calculate 2^refinements, and not refinements^2.

Since this is not a constexpr (as refinements is not known at compile time), you could actually turn it into std::pow, but that's up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'll fix it now.

Copy link
Member

@marcfehling marcfehling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch looks good to me! Thank you for the contribution!

@marcfehling
Copy link
Member

Can you make sure that your commit contains your first and lastname?
See https://github.com/dealii/dealii/wiki/Commit-authorship for more info.

@Darth-Veidim
Copy link
Contributor Author

I've changed the author name for my commits.
Now I have three commits related to one issue. Should I squash them to one using fixup or it may cancel some of the changes?

@marcfehling
Copy link
Member

marcfehling commented Feb 8, 2023

I've changed the author name for my commits. Now I have three commits related to one issue. Should I squash them to one using fixup or it may cancel some of the changes?

Squashing them into one commit using fixup is a good idea. A different strategy would be to do git reset --soft master and create an entirely new commit.

After squashing on your local branch, you can check with git diff darth-branch..origin/darth-branch that they are still identical. You can then proceed to force push.

@marcfehling
Copy link
Member

On a different note: Your master branch is from October 2022, 900 commits behind, which causes two of our testers to fail. Could you also rebase on the current master branch?

@Darth-Veidim Darth-Veidim force-pushed the darth-branch branch 3 times, most recently from 2a9f1e0 to 97599ca Compare February 8, 2023 14:32
@marcfehling
Copy link
Member

Looks like the indentation is off. Could you run make indent from your build directory?
You can also find help here: https://github.com/dealii/dealii/wiki/Indentation

@Darth-Veidim
Copy link
Contributor Author

I fixed this in the last pushed files. It should work fine now.

@bangerth bangerth merged commit 1d44a69 into dealii:master Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants