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

Introduce pre-commit hooks #310

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -244,17 +244,17 @@ jobs:
--- src/combine.cxx 2024-04-23 16:35:27.000000000 +0200
+++ src/combine.cxx.new 2024-07-06 12:29:12.813303074 +0200
@@ -56,12 +56,6 @@
}
-double integral( appl::TH1D* h ) {
}


-double integral( appl::TH1D* h ) {
- double d = 0;
- for ( int i=0 ; i<h->GetNbinsX() ; i++ ) d += h->GetBinContent(i+1);
- return d;
-}
-
void print( appl::TH1D* h ) {

void print( appl::TH1D* h ) {
for ( int i=1 ; i<=h->GetNbinsX() ; i++ ) std::cout << h->GetBinContent(i) << " ";
EOF
# compile static libraries with PIC to make statically linking PineAPPL's CLI work
Expand Down
49 changes: 49 additions & 0 deletions .pre-commit-config.yaml
felixhekhorn marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.6.0
hooks:
- id: trailing-whitespace
exclude: ^pineappl_cli/tests/
- id: end-of-file-fixer
- id: check-merge-conflict
- repo: https://github.com/astral-sh/ruff-pre-commit
# A fast Python linter and code formatter. See
# https://docs.astral.sh/ruff/ for more details.
rev: v0.6.5
hooks:
- id: ruff
args: [--fix]
exclude: ^pineappl_cli/src/plot.py
- id: ruff-format
args: [--line-length=80]
exclude: ^pineappl_cli/src/plot.py
- repo: local
hooks:
- id: fmt
name: cargo fmt
description: Format Rust files with cargo fmt.
entry: cargo fmt --
language: system
files: ^pineappl\S*\/.*\.rs$
args: []
- id: check
Copy link
Contributor

Choose a reason for hiding this comment

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

In eko we have cargo clippy but not cargo check - which one is better? will either fmt or clippy run check along the way?

Copy link
Member

Choose a reason for hiding this comment

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

They are different: cargo check is essentially running the compiler, without generating code. cargo clippy is statically analyzing the code (i.e. the equivalent of Pylint).

There is a non-negligible change that cargo clippy is a superset of cargo check, but it's also possible that they only partially overlap. But this you can search for (or even try: run cargo clippy on something that cargo check was flagging, and see if it's complaining - even a missing ; or unmatched {).
The opposite is definitely impossible: cargo clippy may do strictly more, but certainly not strictly less.

Copy link
Member Author

Choose a reason for hiding this comment

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

They are different: cargo check is essentially running the compiler, without generating code. cargo clippy is statically analyzing the code (i.e. the equivalent of Pylint).

Yes, exactly. I did not include clippy before because it is very noisy but there is actually some ways to suppress warnings and only show errors.

Copy link
Member

Choose a reason for hiding this comment

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

Well: that's certainly an option, but another one is just implementing its suggestions :P

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree! I will address this soon #314 😉

name: cargo check
description: Run cargo check.
entry: bash -c 'cargo check --all-targets --features=evolve,fktable'
language: system
require_serial: true
types: [rust]
- id: clippy
name: cargo clippy
description: Check Rust files with cargo clippy.
# Show only errors and ignore warnings
entry: cargo clippy --all-targets --features=evolve,fktable -- -Awarnings
pass_filenames: false
types: [file, rust]
language: system
- repo: https://github.com/pre-commit/pre-commit
rev: v3.8.0
hooks:
- id: validate_manifest
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

## Rust

- Before you commit, make sure that your code compiles with `cargo check` and
that it has been formatted properly; `cargo fmt` does that for you. Also
check if your changes introduce any new linter warnings by running `cargo
clippy`
- Before you commit, make sure that you have [pre-commit](https://pre-commit.com/)
installed. This will ensure that the code is formatted correctly and that
it compiles properly. Also, check if your changes introduce any new linter
warnings by running `cargo clippy`.
- Make sure to keep `CHANGELOG.md` up-to-date.
- Make sure not to use Rust features newer than the specified minimum supported
Rust Version (MSRV), which is documented in the [README](README.md). You can
Expand Down
1 change: 1 addition & 0 deletions docs/maintainers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
- `_config.yml`: configuration file for PineAPPL's Github-pages website
- `install-capi.sh`: POSIX-compliant shell script to download and install
PineAPPL's pre-built CAPI
- `.pre-commit-config.yaml`: pre-commit hooks configuration

[cargo-xtask]: https://github.com/matklad/cargo-xtask
[release page]: https://github.com/NNPDF/pineappl/releases
Expand Down
1 change: 0 additions & 1 deletion examples/cpp/advanced-filling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,3 @@ int main() {
// release memory
pineappl_grid_delete(grid);
}

1 change: 0 additions & 1 deletion examples/cpp/merge-grids.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,3 @@ int main(int argc, char* argv[]) {
pineappl_grid_delete(clone);
pineappl_grid_delete(grid1);
}

1 change: 0 additions & 1 deletion examples/cpp/modify-grid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,3 @@ int main(int argc, char* argv[]) {
// release memory
pineappl_grid_delete(grid);
}

10 changes: 5 additions & 5 deletions examples/fortran/lhapdf_example.f90
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ program lhapdf_example

call lhapdf_initpdfset_byname(0, "nCTEQ15_1_1")
call lhapdf_initpdfset_byname(1, "nCTEQ15FullNuc_208_82")

! calling pineappl_grid_convolve without any flags
xfx => xfx_test1
alphas => alphas_test1
Expand All @@ -48,7 +48,7 @@ function xfx_test1(pdg_id, x, q2, state) bind(c)
use iso_c_binding

implicit none

integer(c_int32_t), value, intent(in) :: pdg_id
real(c_double), value, intent(in) :: x, q2
type(c_ptr), value, intent(in) :: state
Expand All @@ -61,7 +61,7 @@ function xfx_test2(pdg_id, x, q2, state) bind(c)
use iso_c_binding

implicit none

integer(c_int32_t), value, intent(in) :: pdg_id
real(c_double), value, intent(in) :: x, q2
type(c_ptr), value, intent(in) :: state
Expand All @@ -78,7 +78,7 @@ function alphas_test1(q2, state) bind(c)
use iso_c_binding

implicit none

real(c_double), value, intent(in) :: q2
type(c_ptr), value, intent(in) :: state
real(c_double) :: alphas_test1
Expand All @@ -90,7 +90,7 @@ function alphas_test2(q2, state) bind(c)
use iso_c_binding

implicit none

real(c_double), value, intent(in) :: q2
type(c_ptr), value, intent(in) :: state
real(c_double) :: alphas_test2
Expand Down
20 changes: 10 additions & 10 deletions examples/fortran/pineappl.f90
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function pineappl_alphas(q2, state) bind(c)
use iso_c_binding

implicit none

real(c_double), value, intent(in) :: q2
type (c_ptr), value, intent(in) :: state
real(c_double) :: pineappl_alphas
Expand Down Expand Up @@ -353,15 +353,15 @@ subroutine string_delete(string) bind(c, name = 'pineappl_string_delete')
! https://stackoverflow.com/a/20121335 and https://community.intel.com/t5/Intel-Fortran-Compiler/Converting-c-string-to-Fortran-string/m-p/959515
function c_f_string(c_str) result(f_str)
use :: iso_c_binding

type(c_ptr), intent(in) :: c_str
character(kind=c_char), dimension(:), pointer :: arr_f_ptr => null()
character(len=:, kind=c_char), allocatable :: f_str
integer(kind=c_size_t) :: i, length

length = strlen(c_str)
call c_f_pointer(c_str, arr_f_ptr, [length])

if (.not.associated(arr_f_ptr)) then
f_str = "NULL"
return
Expand All @@ -378,15 +378,15 @@ integer function pineappl_grid_bin_count(grid)
implicit none

type (pineappl_grid), intent(in) :: grid

pineappl_grid_bin_count = grid_bin_count(grid%ptr)
end function

integer function pineappl_grid_bin_dimensions(grid)
implicit none

type (pineappl_grid), intent(in) :: grid

pineappl_grid_bin_dimensions = grid_bin_dimensions(grid%ptr)
end function

Expand Down Expand Up @@ -439,9 +439,9 @@ type (pineappl_grid) function pineappl_grid_clone(grid)

function pineappl_grid_convolve_with_one(grid, pdg_id, xfx, alphas, order_mask, lumi_mask, xi_ren, xi_fac, state) result(res)
use iso_c_binding

implicit none

type (pineappl_grid), intent(in) :: grid
integer, intent(in) :: pdg_id
! no pointer attribute here, see https://community.intel.com/t5/Intel-Fortran-Compiler/Segfault-when-passing-procedure-pointer-to-function-but-not-when/m-p/939797
Expand Down Expand Up @@ -478,9 +478,9 @@ function pineappl_grid_convolve_with_one(grid, pdg_id, xfx, alphas, order_mask,
function pineappl_grid_convolve_with_two(grid, pdg_id1, xfx1, pdg_id2, xfx2, alphas, &
order_mask, lumi_mask, xi_ren, xi_fac, state) result(res)
use iso_c_binding

implicit none

type (pineappl_grid), intent(in) :: grid
integer, intent(in) :: pdg_id1, pdg_id2
procedure (pineappl_xfx) :: xfx1, xfx2
Expand Down Expand Up @@ -570,7 +570,7 @@ function pineappl_grid_key_value(grid, key) result(res)
type (pineappl_grid), intent(in) :: grid
character (*), intent(in) :: key
character (:), allocatable :: res

res = c_f_string(grid_key_value(grid%ptr, key // c_null_char))
end function

Expand Down
8 changes: 4 additions & 4 deletions examples/fortran/test.f90
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
program test_pineappl
use pineappl
use iso_c_binding

implicit none

integer, parameter :: dp = kind(0.0d0)
Expand Down Expand Up @@ -164,7 +164,7 @@ function xfx1_test(pdg_id, x, q2, state) bind(c)
use iso_c_binding

implicit none

integer(c_int32_t), value, intent(in) :: pdg_id
real(c_double), value, intent(in) :: x, q2
type(c_ptr), value, intent(in) :: state
Expand All @@ -177,7 +177,7 @@ function xfx2_test(pdg_id, x, q2, state) bind(c)
use iso_c_binding

implicit none

integer(c_int32_t), value, intent(in) :: pdg_id
real(c_double), value, intent(in) :: x, q2
type(c_ptr), value, intent(in) :: state
Expand All @@ -190,7 +190,7 @@ function alphas_test(q2, state) bind(c)
use iso_c_binding

implicit none

real(c_double), value, intent(in) :: q2
type(c_ptr), value, intent(in) :: state
real(c_double) :: alphas_test
Expand Down
62 changes: 34 additions & 28 deletions pineappl_cli/src/subgrid-pull-plot.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,58 +5,64 @@
from math import fabs, log10
from scipy.interpolate import griddata

x1 = np.array([{x1}])
x2 = np.array([{x2}])
z = np.array([{z}])
x1 = np.array([{x1}]) # noqa: F821
x2 = np.array([{x2}]) # noqa: F821
z = np.array([{z}]) # noqa: F821
x = 0.5 * np.log(x1 / x2)
y = np.sqrt(x1 * x2)

nrap = 50
nmass = 50

sym_min = -max(fabs(np.min(x)), fabs(np.max(x)))
sym_max = max(fabs(np.min(x)), fabs(np.max(x)))
sym_max = max(fabs(np.min(x)), fabs(np.max(x)))

xi = np.linspace(sym_min, sym_max, (nrap // 2) * 2 + 1)
yi = np.logspace(log10(np.min(y)), log10(np.max(y)), nmass)
zi = griddata((x, y), z, (xi[None, :], yi[:, None]), method='linear', rescale=True)
zi = griddata(
(x, y), z, (xi[None, :], yi[:, None]), method="linear", rescale=True
)

#print(xi.shape)
#print(yi.shape)
#print(zi.shape)
# print(xi.shape)
# print(yi.shape)
# print(zi.shape)

# mask impossible kinematic values
for iy, ix in np.ndindex(zi.shape):
#print(ix, iy)
# print(ix, iy)
x1v = yi[iy] * np.exp(xi[ix])
x2v = yi[iy] / np.exp(xi[ix])

#print('y = {{}} m/s = {{}} -> x1 = {{}} x2 = {{}}'.format(xi[ix], yi[iy], x1v, x2v))
# print('y = {{}} m/s = {{}} -> x1 = {{}} x2 = {{}}'.format(xi[ix], yi[iy], x1v, x2v))

if x1v > 1.0 or x2v > 1.0:
zi[iy, ix] = np.nan

figure, axes = plt.subplots(1, 2, constrained_layout=True)
figure.set_size_inches(10, 5)

mesh = axes[0].pcolormesh(xi, yi, zi, shading='nearest', linewidth=0, snap=True)
axes[0].scatter(x, y, marker='*', s=5)
axes[0].set_yscale('log')
axes[0].set_xlabel(r'$y = 1/2 \log (x_1/x_2)$')
axes[0].set_ylabel(r'$M/\sqrt{{s}} = \sqrt{{x_1 x_2}}$')
#axes[0].set_aspect('equal', share=True)
mesh = axes[0].pcolormesh(xi, yi, zi, shading="nearest", linewidth=0, snap=True)
axes[0].scatter(x, y, marker="*", s=5)
axes[0].set_yscale("log")
axes[0].set_xlabel(r"$y = 1/2 \log (x_1/x_2)$")
axes[0].set_ylabel(r"$M/\sqrt{{s}} = \sqrt{{x_1 x_2}}$")
# axes[0].set_aspect('equal', share=True)

x1i = np.logspace(log10(np.min(x1)), log10(np.max(x1)), 50)
x2i = np.logspace(log10(np.min(x2)), log10(np.max(x2)), 50)
z12i = griddata((x1, x2), z, (x1i[None, :], x2i[:, None]), method='linear', rescale=True)

mesh = axes[1].pcolormesh(x1i, x2i, z12i, shading='nearest', linewidth=0, snap=True)
axes[1].set_xscale('log')
axes[1].set_yscale('log')
axes[1].scatter(x1, x2, marker='*', s=5)
axes[1].set_aspect('equal', share=True)
axes[1].set_xlabel(r'$x_1$')
axes[1].set_ylabel(r'$x_2$')

figure.colorbar(mesh, ax=axes, extend='min')
figure.savefig('plot.pdf')
z12i = griddata(
(x1, x2), z, (x1i[None, :], x2i[:, None]), method="linear", rescale=True
)

mesh = axes[1].pcolormesh(
x1i, x2i, z12i, shading="nearest", linewidth=0, snap=True
)
axes[1].set_xscale("log")
axes[1].set_yscale("log")
axes[1].scatter(x1, x2, marker="*", s=5)
axes[1].set_aspect("equal", share=True)
axes[1].set_xlabel(r"$x_1$")
axes[1].set_ylabel(r"$x_2$")

figure.colorbar(mesh, ax=axes, extend="min")
figure.savefig("plot.pdf")
Loading