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 triangle to convexity helper #1717

Merged

Conversation

JacquesOlivierLachaud
Copy link
Member

@JacquesOlivierLachaud JacquesOlivierLachaud commented Feb 20, 2024

PR Description

Add specific methods in ConvexityHelper and DigitalConvexity to build lattice polytope from 2 or 3 points, even in degenerate cases. This speeds up their construction by a factor 3 to 5.

Checklist

  • Unit-test of your feature with Catch.
  • Doxygen documentation of the code completed (classes, methods, types, members...)
  • Documentation module page added or updated.
  • New entry in the ChangeLog.md added.
  • No warning raised in Debug mode.
  • All continuous integration tests pass (Github Actions)

@JacquesOlivierLachaud
Copy link
Member Author

PythonTests fail but it seems unrelated to this PR.

Copy link
Member

@dcoeurjo dcoeurjo left a comment

Choose a reason for hiding this comment

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

Looks good to me, just a minor comment

ChangeLog.md Outdated Show resolved Hide resolved
@dcoeurjo
Copy link
Member

dcoeurjo commented Mar 3, 2024

It looks like "testConvexityHelper" fails in the CI bots.

@JacquesOlivierLachaud
Copy link
Member Author

Curiously it is working on my Mac Debug/Release. Do you have an ubuntu to tell me what's happening ?

@dcoeurjo
Copy link
Member

On a linux docker image:


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
testConvexityHelper is a Catch v2.13.7 host application.
Run with -? for options

-------------------------------------------------------------------------------
Scenario: ConvexityHelper< 3 > triangle tests
      Given: Given non degenerated triplets of points
       When: Computing their tightiest polytope or triangle, dilated by a cube
       Then: They have the same number of interior points
-------------------------------------------------------------------------------
/DGtal/tests/geometry/volumes/testConvexityHelper.cpp:528
...............................................................................

/DGtal/tests/geometry/volumes/testConvexityHelper.cpp:529: FAILED:
  REQUIRE( nbi_ok == nb_total )
with expansion:
  18 == 19
with messages:
  a := (4, 3, 9)
  b := (6, 4, 6)
  c := (7, 4, 6)
  n := (0, -3, -1)
  P := [BoundedLatticePolytope<3> A.rows=12 valid_edge_constraints=1]
    [ 1 0 0 ] . x <= 8
    [ -1 0 0 ] . x <= -4
    [ 0 1 0 ] . x <= 5
    [ 0 -1 0 ] . x <= -3
    [ 0 0 1 ] . x <= 10
    [ 0 0 -1 ] . x <= -6
    [ 0 -3 -1 ] . x <= -18
    [ 0 3 1 ] . x <= 22
    [ -3 0 -2 ] . x <= -30
    [ -1 2 0 ] . x <= 4
    [ 3 0 3 ] . x <= 45
    [ 1 -3 0 ] . x <= -4
  T := [BoundedLatticePolytope<3> A.rows=14 valid_edge_constraints=1]
    [ 1 0 0 ] . x <= 8
    [ -1 0 0 ] . x <= -4
    [ 0 1 0 ] . x <= 5
    [ 0 -1 0 ] . x <= -3
    [ 0 0 1 ] . x <= 10
    [ 0 0 -1 ] . x <= -6
    [ 0 -3 -1 ] . x <= -18
    [ 0 3 1 ] . x <= 22
    [ -3 0 -2 ] . x <= -30
    [ 0 0 -1 ] . x <= -6
    [ 3 0 3 ] . x <= 45
    [ -1 2 0 ] . x <= 4
    [ 0 1 0 ] . x <= 5
    [ 1 -3 0 ] . x <= -4
  nb_P := 24
  nb_T := 24
  nbi_P := 8
  nbi_T := 6

===============================================================================
test cases:   4 |   3 passed | 1 failed
assertions: 103 | 102 passed | 1 failed

@dcoeurjo
Copy link
Member

Hi @JacquesOlivierLachaud , were you able to reproduce the bug ?

@JacquesOlivierLachaud
Copy link
Member Author

Sorry I haven't found any time to work on it (and I forgot at some point :) ) . I will come back to it soon.

@dcoeurjo
Copy link
Member

dcoeurjo commented Jun 1, 2024

I'm going to create a DGtal release, would you like this PR to be included?

@JacquesOlivierLachaud
Copy link
Member Author

What is the deadline ? I might be able to do it next week.

@dcoeurjo
Copy link
Member

dcoeurjo commented Jun 3, 2024

I'd say by Wednesday ?

@JacquesOlivierLachaud
Copy link
Member Author

When I build it on linux noble docker image in Debug Mode, it passes ! Only testLinearStructure fails:

Output:
----------------------------------------------------------
^[[0m^[[1mNew Block [testing 3d operators]^[[0m
  [dec | primal 0-cells <-> dual 3-cells (2) | primal 1-cells <-> dual 2-cells (5) | primal 2-cells <-> dual 1-cells (4) | primal 3-cells <-> dual 0-cells (1)]
  ^[[0m^[[1mNew Block [base operators]^[[0m
    d0
[primal 0-form => primal 1-form (2 => 5)]
 1  0
 0 -1
 0 -1
 1 -1
 1  0
    d2p
[dual 2-form => dual 3-form (5 => 2)]
 1  0  0  1  1
 0 -1 -1 -1  0
    ^[[0m^[[31m[ERR] Fatal Error - assertion (Eigen::MatrixXd(d0.myContainer) == d0_th) failed in void test_manual_operators_3d(): /tmp/DGtal/tests/dec/testLinearStructure.cpp(391)
<end of output>
Test time =   0.00 sec

@dcoeurjo
Copy link
Member

dcoeurjo commented Jun 6, 2024

👌 let me try again (I was able to reproduce the bug)

@JacquesOlivierLachaud
Copy link
Member Author

Oups, I have made a mistake. I think I am not on the correct branch.

@JacquesOlivierLachaud
Copy link
Member Author

Sorry. I can reproduce it. I try to look at it today (but tonight probably).

@dcoeurjo
Copy link
Member

dcoeurjo commented Jun 6, 2024

ok great. Here you have my trace:
https://asciinema.org/a/RYYW8Mm3iupP70Jo01zuP6lmK

@dcoeurjo
Copy link
Member

dcoeurjo commented Jun 6, 2024

(ubuntu jammy)

@JacquesOlivierLachaud
Copy link
Member Author

It looks like the other errors are unrelated to this PR.

@dcoeurjo dcoeurjo merged commit 20c1d41 into DGtal-team:master Jun 8, 2024
8 of 10 checks passed
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.

2 participants