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

Format everything & delete old bot comments #674

Merged
merged 3 commits into from
Dec 3, 2020
Merged
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
8 changes: 8 additions & 0 deletions .github/bot-pr-base.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ HEAD_URL="https://${GITHUB_ACTOR}:${GITHUB_TOKEN}@github.com/$HEAD_REPO"

JOB_URL="https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"

bot_delete_comments_matching() {
local search_matching="$1"
COMMENTS=$(api_get "$ISSUE_URL/comments" | jq -r '.[] | select((.user.login == "ginkgo-bot") and (.body | startswith('"\"$search_matching\""'))) | .url')
for URL in $COMMENTS; do
api_delete "$URL" > /dev/null
done
}

bot_comment() {
api_post "$ISSUE_URL/comments" "{\"body\":\"$1\"}" > /dev/null
}
Expand Down
4 changes: 2 additions & 2 deletions .github/check-format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ LIST_FILES=$(git diff --name-only | sed '$!s/$/\\n/' | tr -d '\n')
git diff > /tmp/format.patch
mv /tmp/format.patch .

bot_delete_comments_matching "Error: The following files need to be formatted"

if [[ "$LIST_FILES" != "" ]]; then
MESSAGE="The following files need to be formatted:\n"'```'"\n$LIST_FILES\n"'```'
MESSAGE="$MESSAGE\nYou can find a formatting patch under **Artifacts** [here]"
MESSAGE="$MESSAGE($JOB_URL) or run "'`format!` if you have write access to Ginkgo'
bot_error "$MESSAGE"
else
bot_comment "No formatting necessary"
fi
2 changes: 0 additions & 2 deletions .github/format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,4 @@ if [[ "$LIST_FILES" != "" ]]; then

Co-authored-by: $USER_COMBINED"
git push fork $LOCAL_BRANCH:$HEAD_BRANCH 2>&1 || bot_error "Cannot push formatted branch, are edits for maintainers allowed?"
else
bot_comment "No formatting necessary"
fi
2 changes: 2 additions & 0 deletions .github/rebase.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ git config user.name "$USER_NAME"
LOCAL_BRANCH=rebase-tmp-$HEAD_BRANCH
git checkout -b $LOCAL_BRANCH fork/$HEAD_BRANCH

bot_delete_comments_matching "Error: Rebase failed"

# do the rebase
git rebase base/$BASE_BRANCH 2>&1 || bot_error "Rebase failed, see the related [Action]($JOB_URL) for details"

Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/bot-pr-comment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
fetch-depth: 0
- name: Add appropriate labels
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between secrets.GITHUB_TOKEN and secrets.BOT_TOKEN?

Copy link
Member Author

@upsj upsj Dec 3, 2020

Choose a reason for hiding this comment

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

GITHUB_TOKEN is an automatically generated and temporary token for github-actions[bot], which has the annoying property that pushes from this token don't trigger subsequent Github Actions (like Windows/macOS CI jobs) to avoid infinite loops in Github Actions by default.
BOT_TOKEN is a manually generated personal authentication token for @ginkgo-bot (stored in this repo's Secrets) which circumvents this limitation.
So in the future, comments will come from ginkgo-bot instead of github-actions[bot] and the CI jobs will run after every push

run: cp .github/label.sh /tmp && /tmp/label.sh
check_format:
name: check-format
Expand All @@ -28,7 +28,7 @@ jobs:
fetch-depth: 0
- name: Check for formatting changes
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
run: cp .github/check-format.sh /tmp && /tmp/check-format.sh
- name: Upload code formatting patch
if: failure()
Expand All @@ -48,7 +48,7 @@ jobs:
fetch-depth: 0
- name: Commit formatting changes
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
run: cp .github/format.sh /tmp && /tmp/format.sh
rebase:
name: rebase
Expand All @@ -62,5 +62,5 @@ jobs:
fetch-depth: 0
- name: Automatic Rebase
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
run: cp .github/rebase.sh /tmp && /tmp/rebase.sh
2 changes: 1 addition & 1 deletion .github/workflows/bot-pr-created.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,5 @@ jobs:
fetch-depth: 0
- name: Add appropriate labels
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
run: .github/label.sh
2 changes: 1 addition & 1 deletion .github/workflows/bot-pr-updated.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
fetch-depth: 0
- name: Check for formatting changes
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_TOKEN: ${{ secrets.BOT_TOKEN }}
run: cp .github/check-format.sh /tmp && /tmp/check-format.sh
- name: Upload code formatting patch
if: failure()
Expand Down
1 change: 0 additions & 1 deletion benchmark/solver/solver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "benchmark/utils/loggers.hpp"
#include "benchmark/utils/overhead_linop.hpp"
#include "benchmark/utils/preconditioners.hpp"
#include "ginkgo/core/preconditioner/isai.hpp"


// some Ginkgo shortcuts
Expand Down
1 change: 0 additions & 1 deletion benchmark/utils/preconditioners.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
******************************<GINKGO LICENSE>*******************************/


#ifndef GKO_BENCHMARK_UTILS_PRECONDITIONERS_HPP_
#define GKO_BENCHMARK_UTILS_PRECONDITIONERS_HPP_

Expand Down
2 changes: 1 addition & 1 deletion core/base/allocator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,4 @@ using unordered_map =

} // namespace gko

#endif // GKO_CORE_BASE_ALLOCATOR_HPP_
#endif // GKO_CORE_BASE_ALLOCATOR_HPP_
13 changes: 7 additions & 6 deletions core/base/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
******************************<GINKGO LICENSE>*******************************/

#ifndef GKO_INTERNAL_CORE_BASE_UTILS_HPP_
#define GKO_INTERNAL_CORE_BASE_UTILS_HPP_
#ifndef GKO_CORE_BASE_UTILS_HPP_
#define GKO_CORE_BASE_UTILS_HPP_


#include <ginkgo/core/base/utils.hpp>


#include <memory>
Expand All @@ -40,7 +43,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#include <ginkgo/core/base/polymorphic_object.hpp>
#include <ginkgo/core/base/types.hpp>
#include <ginkgo/core/base/utils.hpp>
#include <ginkgo/core/matrix/csr.hpp>


Expand All @@ -64,8 +66,7 @@ namespace detail {


template <typename Dest>
struct conversion_sort_helper {
};
struct conversion_sort_helper {};

template <typename ValueType, typename IndexType>
struct conversion_sort_helper<matrix::Csr<ValueType, IndexType>> {
Expand Down Expand Up @@ -222,4 +223,4 @@ std::shared_ptr<const Dest> convert_to_with_sorting(
} // namespace gko


#endif // GKO_INTERNAL_CORE_BASE_UTILS_HPP_
#endif // GKO_CORE_BASE_UTILS_HPP_
1 change: 0 additions & 1 deletion core/device_hooks/dpcpp_hooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
******************************<GINKGO LICENSE>*******************************/


#include <memory>
#include <string>

Expand Down
4 changes: 3 additions & 1 deletion core/preconditioner/isai_kernels.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define GKO_CORE_PRECONDITIONER_ISAI_KERNELS_HPP_


#include <ginkgo/core/matrix/csr.hpp>
#include <ginkgo/core/preconditioner/isai.hpp>


#include <ginkgo/core/matrix/csr.hpp>


namespace gko {
namespace kernels {

Expand Down
1 change: 1 addition & 0 deletions core/test/utils/unsort_matrix.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#ifndef GKO_CORE_TEST_UTILS_UNSORT_MATRIX_HPP_
#define GKO_CORE_TEST_UTILS_UNSORT_MATRIX_HPP_


#include <algorithm>
#include <random>

Expand Down
6 changes: 3 additions & 3 deletions cuda/base/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define GKO_CUDA_BASE_TYPES_HPP_


#include <ginkgo/core/base/types.hpp>


#include <type_traits>


Expand All @@ -43,9 +46,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <thrust/complex.h>


#include <ginkgo/core/base/types.hpp>


namespace gko {


Expand Down
1 change: 1 addition & 0 deletions cuda/factorization/factorization_kernels.cu
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

#include <ginkgo/core/base/array.hpp>


#include "core/components/prefix_sum.hpp"
#include "core/matrix/csr_builder.hpp"
#include "cuda/base/config.hpp"
Expand Down
1 change: 0 additions & 1 deletion cuda/factorization/par_ilut_approx_filter_kernel.cu
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.


#include "core/components/prefix_sum.hpp"
#include "core/factorization/par_ilut_kernels.hpp"
Copy link
Member

Choose a reason for hiding this comment

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

Is this maybe necessary ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was as surprised as you are: This is a duplicate of the topmost header

#include "core/matrix/coo_builder.hpp"
#include "core/matrix/csr_builder.hpp"
#include "core/matrix/csr_kernels.hpp"
Expand Down
2 changes: 1 addition & 1 deletion cuda/factorization/par_ilut_filter_kernel.cu
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,4 @@ GKO_INSTANTIATE_FOR_EACH_VALUE_AND_INDEX_TYPE(
} // namespace par_ilut_factorization
} // namespace cuda
} // namespace kernels
} // namespace gko
} // namespace gko
2 changes: 1 addition & 1 deletion cuda/solver/idr_kernels.cu
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "core/solver/idr_kernels.hpp"


#include <time.h>
#include <random>
#include <time.h>


#include <ginkgo/core/base/exception_helpers.hpp>
Expand Down
1 change: 0 additions & 1 deletion cuda/test/reorder/rcm_kernels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
******************************<GINKGO LICENSE>*******************************/


#include <ginkgo/core/reorder/rcm.hpp>


Expand Down
16 changes: 16 additions & 0 deletions dev_tools/scripts/config
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,26 @@
- FixInclude: "hip/hip_runtime.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean hip_runtime? It contains all the macros like hipLaunchKernelGGL, HIP_KERNEL_NAME, device-side assert etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I meant this entire config file, haha. I guess it's unrelated to this PR but I have no idea what's going on here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This file configures the format_header.sh script by fixing a few special-cases where the default detection of the main header belonging to a file (the one that is included at the top) doesn't work properly

- "(cuda|hip|omp|dpcpp)/test/factorization/par_ilu_kernels"
- FixInclude: "core/factorization/par_ilu_kernels.hpp"
- "(cuda|hip|omp|dpcpp)/test/factorization/par_ilut_kernels"
- FixInclude: "core/factorization/par_ilut_kernels.hpp"
- "(cuda|hip|omp|dpcpp)/test/factorization/par_ict_kernels"
- FixInclude: "core/factorization/par_ict_kernels.hpp"
- "cuda/factorization/par_ilut_select_common"
- FixInclude: "cuda/factorization/par_ilut_select_common.cuh"
- "hip/factorization/par_ilut_select_common"
- FixInclude: "hip/factorization/par_ilut_select_common.hip.hpp"
- "(cuda|hip|dpcpp)/factorization/par_ilut_"
- FixInclude: "core/factorization/par_ilut_kernels.hpp"
- "(cuda|hip|dpcpp)/factorization/par_ict_"
- FixInclude: "core/factorization/par_ict_kernels.hpp"
- "(cuda|hip|dpcpp)/preconditioner/jacobi_"
- FixInclude: "core/preconditioner/jacobi_kernels.hpp"
- "core/test/base/(extended_float|iterator_factory)"
- RemoveTest: "true"
- "core/test/base/allocator"
- FixInclude: "core/base/allocator.hpp"
- "reference/test/base/utils"
- FixInclude: "core/base/utils.hpp"
Copy link
Member

Choose a reason for hiding this comment

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

at some point, I will check whether adding a special comment as the main header.

#include "core/base/utils.hpp" // MAIN

it will use it as main header no matter how the config sets

- "_builder\.cpp"
- RemoveTest: "true"
- "_builder\.hpp"
Expand Down
2 changes: 1 addition & 1 deletion dev_tools/scripts/regroup
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ IncludeBlocks: Regroup
IncludeCategories:
- Regex: '^<(rapidjson|gflags|gtest|papi).*'
Priority: 3
- Regex: '^<(omp|cu|hip|thrust|CL).*'
- Regex: '^<(omp|cu|hip|thrust|CL/|cooperative).*'
Priority: 2
- Regex: '^<ginkgo.*'
Priority: 5
Expand Down
3 changes: 2 additions & 1 deletion examples/ginkgo-overhead/ginkgo-overhead.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include <iostream>


[[noreturn]] void print_usage_and_exit(const char *name) {
[[noreturn]] void print_usage_and_exit(const char *name)
{
std::cerr << "Usage: " << name << " [NUM_ITERS]" << std::endl;
std::exit(-1);
}
Expand Down
4 changes: 2 additions & 2 deletions hip/base/types.hip.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#define GKO_HIP_BASE_TYPES_HIP_HPP_


#include <type_traits>
#include <ginkgo/core/base/types.hpp>


#include <ginkgo/core/base/types.hpp>
#include <type_traits>


#include <hip/hip_complex.h>
Expand Down
5 changes: 2 additions & 3 deletions hip/factorization/par_ilut_approx_filter_kernel.hip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "core/factorization/par_ilut_kernels.hpp"


#include <hip/hip_runtime.h>
#include <algorithm>


#include <algorithm>
#include <hip/hip_runtime.h>


#include <ginkgo/core/base/array.hpp>
Expand All @@ -47,7 +47,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.


#include "core/components/prefix_sum.hpp"
#include "core/factorization/par_ilut_kernels.hpp"
#include "core/matrix/coo_builder.hpp"
#include "core/matrix/csr_builder.hpp"
#include "core/matrix/csr_kernels.hpp"
Expand Down
2 changes: 1 addition & 1 deletion hip/factorization/par_ilut_filter_kernel.hip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,4 @@ GKO_INSTANTIATE_FOR_EACH_VALUE_AND_INDEX_TYPE(
} // namespace par_ilut_factorization
} // namespace hip
} // namespace kernels
} // namespace gko
} // namespace gko
4 changes: 2 additions & 2 deletions hip/factorization/par_ilut_select_kernel.hip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "core/factorization/par_ilut_kernels.hpp"


#include <hip/hip_runtime.h>
#include <algorithm>


#include <algorithm>
#include <hip/hip_runtime.h>


#include <ginkgo/core/base/array.hpp>
Expand Down
2 changes: 1 addition & 1 deletion hip/solver/idr_kernels.hip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "core/solver/idr_kernels.hpp"


#include <time.h>
#include <random>
#include <time.h>


#include <hip/hip_runtime.h>
Expand Down
6 changes: 3 additions & 3 deletions include/ginkgo/core/base/abstract_factory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
******************************<GINKGO LICENSE>*******************************/

#ifndef GKO_CORE_BASE_ABSTRACT_FACTORY_HPP_
#define GKO_CORE_BASE_ABSTRACT_FACTORY_HPP_
#ifndef GKO_PUBLIC_CORE_BASE_ABSTRACT_FACTORY_HPP_
#define GKO_PUBLIC_CORE_BASE_ABSTRACT_FACTORY_HPP_


#include <ginkgo/core/base/polymorphic_object.hpp>
Expand Down Expand Up @@ -289,4 +289,4 @@ struct enable_parameters_type {
} // namespace gko


#endif // GKO_CORE_BASE_ABSTRACT_FACTORY_HPP_
#endif // GKO_PUBLIC_CORE_BASE_ABSTRACT_FACTORY_HPP_
6 changes: 3 additions & 3 deletions include/ginkgo/core/base/array.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
******************************<GINKGO LICENSE>*******************************/

#ifndef GKO_CORE_BASE_ARRAY_HPP_
#define GKO_CORE_BASE_ARRAY_HPP_
#ifndef GKO_PUBLIC_CORE_BASE_ARRAY_HPP_
#define GKO_PUBLIC_CORE_BASE_ARRAY_HPP_


#include <algorithm>
Expand Down Expand Up @@ -619,4 +619,4 @@ class copy_back_deleter<Array<T>> {
} // namespace gko


#endif // GKO_CORE_BASE_ARRAY_HPP_
#endif // GKO_PUBLIC_CORE_BASE_ARRAY_HPP_
Loading