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

Add build and tests on Linux, Mac, and Windows to GitHub CI #467

Merged
merged 18 commits into from
Mar 16, 2023

Conversation

SSoelvsten
Copy link
Owner

@SSoelvsten SSoelvsten commented Mar 14, 2023

This addition is heavily based on my contribution to TPIE. Adding this now also exposes me to (new) compilation issues with Clang (and MSVC). Hence, this also is a follow up on #403 and #317 .

Bug Fixes

  • Fix ordering of substitute and reduce in the statistics struct. This explains why said value might be unexpectedly 0.

Clang

src/adiar

Warnings

  • algorithms/reduce.h:39:5: Definition of implicit copy assignment operator for 'reduce_arc' is deprecated because it has a user-declared copy constructor.

  • */count.*: warning: suggest braces around initialization of subobject

    I do not think, these can be fixed, since the extra braces conflict with the simpler path-based version. The real solution is to switch to the request struct.

  • algorithms/substitution.h:33:5: warning: definition of implicit copy assignment operator for 'substitute_arc'

Errors / Test Failures

  • request class template
    • Cannot use copy-constructor for an array.
    • It seemed like, a zero-sized array does not take up 0 bytes (on MacOS only?). It essentially treats int[] the way GCC treats std::array<int>. Yet, I cannot reproduce this. Until it shows up again, I will not fix it.

test/adiar

Warnings

  • io/test_file.cpp:122:49: warning: lambda capture 'curr_path' is not used
  • io/test_levelized_file.cpp:855:95: warning: lambda capture 'curr_path' is not used
  • io/test_levelized_file.cpp:1279:45: warning: lambda capture 'tmp_path' is not used
  • io/test_levelized_file.cpp:1290:45: warning: lambda capture 'tmp_path' is not used
  • io/test_levelized_file.cpp:1301:50: warning: lambda capture 'tmp_path' is not used

Windows

src/adiar

Errors

  • The value TRUE, FALSE, or NONE (or more?) are already defined in the preprocessor.

  • internal/io/node_writer.h(362,48): error C2672: 'std::max': no matching overloaded function found

  • internal/data_types/request.h(212,3): error C2503: 'adiar::internal::request<2,0,1>': base classes cannot contain zero-sized arrays

    • internal/algorithms/quantify.h(94,85): error C2988: unrecognizable template declaration/definition
    • internal/algorithms/quantify.h(94,1): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
  • bdd\apply.cpp(59,7): error C3861: '__builtin_unreachable': identifier not found

  • internal/algorithms/reduce.h(279,9) 'adiar::internal::__reduce_cut_add': no overloaded function could convert all the argument types

  • bdd\build.cpp(49,33)/ (50,3): missing type specifier - int assumed. Note: C++ does not support default-int

  • bdd\build.cpp(49,15)/ (86,12)/ (87,12): error C2146: syntax error: missing ';' before identifier

  • Lots of issues with CNL

    • call to object of class type 'cnl::post_operator<Operator,cnl::wide_tag<128,uint32_t,void>,cnl::_impl::duplex_integer<cnl::_impl::duplex_integer<unsigned int,unsigned int>,cnl::_impl::duplex_integer<unsigned int,unsigned int>>,void>': no matching call operator found
      • bdd\apply.cpp(108,63)
      • ? (?,?)
      • bdd\restrict.cpp(65,62):
      • zdd\binop.cpp(143,53)
      • zdd\change.cpp(57,60)
      • zdd\complement.cpp(75,66)
      • zdd\expand.cpp(59,60)
      • zdd\subset.cpp(112,108)
    • 'cnl::_impl::from_rep': no matching overloaded function found
      • bdd/bdd.cpp(51,71): message : see reference to function template instantiation 'adiar::bdd adiar::internal::reduceadiar::bdd_policy(const adiar::__bdd &)' being compiled

Warnings

  • statistics.cpp(55,32): warning C4129: 'e': unrecognized character escape sequence
  • test_request.cpp(22,9): '==': unsafe mix of type 'const ExpectedType' and type 'const ActualType' in operation

@SSoelvsten SSoelvsten added the 📁 build CMake, GitHub Actions etc. label Mar 14, 2023
@SSoelvsten SSoelvsten self-assigned this Mar 14, 2023
@SSoelvsten SSoelvsten force-pushed the ci/check-all-platforms branch from c9e41bf to 849ab13 Compare March 14, 2023 13:59
@SSoelvsten SSoelvsten added the 🔥 bug Something isn't working label Mar 14, 2023
@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Patch coverage: 88.660% and project coverage change: +0.005 🎉

Comparison is base (659bae1) 97.165% compared to head (1901521) 97.170%.

Additional details and impacted files
@@              Coverage Diff              @@
##              main      #467       +/-   ##
=============================================
+ Coverage   97.165%   97.170%   +0.005%     
=============================================
  Files           80        80               
  Lines         4480      4488        +8     
=============================================
+ Hits          4353      4361        +8     
  Misses         127       127               
Impacted Files Coverage Δ
src/adiar/cube.h 100.000% <ø> (ø)
src/adiar/internal/cut.h 100.000% <ø> (ø)
src/adiar/statistics.cpp 86.905% <ø> (ø)
src/adiar/statistics.h 100.000% <ø> (ø)
src/adiar/internal/algorithms/count.h 90.000% <33.333%> (ø)
src/adiar/internal/algorithms/substitution.h 94.318% <60.000%> (ø)
src/adiar/internal/algorithms/intercut.h 98.198% <66.667%> (ø)
src/adiar/internal/algorithms/pred.h 95.506% <75.000%> (ø)
src/adiar/internal/algorithms/quantify.h 97.838% <75.000%> (ø)
src/adiar/bdd/if_then_else.cpp 96.140% <80.000%> (ø)
... and 11 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@SSoelvsten SSoelvsten force-pushed the ci/check-all-platforms branch 4 times, most recently from ace4265 to 7164b0b Compare March 14, 2023 17:00
Repository owner deleted a comment from github-actions bot Mar 15, 2023
Repository owner deleted a comment from github-actions bot Mar 15, 2023
Repository owner deleted a comment from github-actions bot Mar 15, 2023
@SSoelvsten SSoelvsten force-pushed the ci/check-all-platforms branch 2 times, most recently from e1958ee to 9372ef1 Compare March 15, 2023 08:25
@SSoelvsten SSoelvsten force-pushed the ci/check-all-platforms branch 5 times, most recently from 1f4f42a to 35261cf Compare March 15, 2023 11:06
Repository owner deleted a comment from github-actions bot Mar 15, 2023
@SSoelvsten SSoelvsten force-pushed the ci/check-all-platforms branch 2 times, most recently from 86733b6 to b3d0777 Compare March 15, 2023 12:44
Repository owner deleted a comment from github-actions bot Mar 15, 2023
Repository owner deleted a comment from github-actions bot Mar 15, 2023
@SSoelvsten SSoelvsten force-pushed the ci/check-all-platforms branch 3 times, most recently from f0db089 to d528a60 Compare March 15, 2023 13:21
@SSoelvsten SSoelvsten force-pushed the ci/check-all-platforms branch from d528a60 to 123e983 Compare March 15, 2023 13:48
@SSoelvsten SSoelvsten force-pushed the ci/check-all-platforms branch from 123e983 to 1901521 Compare March 15, 2023 13:52
@github-actions
Copy link

Benchmark Report 🟢

origin/ci/check-all-platforms is a regression of 0.35% (compared to origin/main).

Minimum running time (s) for 12-Queens:

origin/main origin/ci/check-all-platforms
18.12 18.18
Raw Data

Running times (s) for 12-Queens:

origin/main origin/ci/check-all-platforms
18.26 18.31
18.12 18.26
18.19 18.18
18.12 18.18
18.16 18.23
18.18 18.20
18.16 18.23
18.20 18.31
18.22 18.33
18.17 18.20
18.14 18.26
18.19 18.32
18.14 18.19
18.19 18.28
18.19 18.28
18.23 18.26

@github-actions
Copy link

Benchmark Report 🟢

origin/ci/check-all-platforms is an improvement of 0.00% (compared to origin/main).

Minimum running time (s) for 9-Queens:

origin/main origin/ci/check-all-platforms
0.32 0.32
Raw Data

Running times (s) for 9-Queens:

origin/main origin/ci/check-all-platforms
0.32 0.35
0.37 0.32
0.35 0.36
0.35 0.35
0.34 0.32
0.35 0.38
0.36 0.32
0.39 0.33
0.37 0.32
0.33 0.37
0.37 0.35
0.32 0.32
0.34 0.33
0.32 0.33
0.43 0.33
0.32 0.32

@github-actions
Copy link

Benchmark Report 🟢

origin/ci/check-all-platforms is a regression of 0.61% (compared to origin/main).

Minimum running time (s) for 14-Queens:

origin/main origin/ci/check-all-platforms
619.29 623.05
Raw Data

Running times (s) for 14-Queens:

origin/main origin/ci/check-all-platforms
758.42 635.83
626.92 625.39
619.70 624.65
619.29 623.05

@SSoelvsten SSoelvsten merged commit 1a6463b into main Mar 16, 2023
@SSoelvsten SSoelvsten deleted the ci/check-all-platforms branch March 16, 2023 07:42
@SSoelvsten SSoelvsten restored the ci/check-all-platforms branch March 16, 2023 15:03
@SSoelvsten SSoelvsten deleted the ci/check-all-platforms branch March 16, 2023 15:03
@SSoelvsten SSoelvsten restored the ci/check-all-platforms branch March 16, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔥 bug Something isn't working 📁 build CMake, GitHub Actions etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant