siddhartha-gadgil / Superficial

Curves and other structures on surfaces (topology)
MIT License
3 stars 13 forks source link

Some path is causing an error in shorten #83

Closed shabarishch closed 3 years ago

shabarishch commented 3 years ago

The function shorten moves a PLPath to the local minimum for length in its isotopy class. It now takes in two additional parameters -

  1. uniqrepuptoflipandcyclicper - a map which gives, for each normal path, its unique representative upto flips and cyclic permutations that has been enumerated into a PLPath
  2. enumdata - a map defined on the set of normalpaths in the image of uniqrepuptoflipandcyclicper that takes a normalpath to its minimal length plpath.

For shortening a PLPath of base length n, we need enumdata for paths of base length upto (n+1) since shifting across a vertex may increase base length by 1. The attached worksheet presents an error on the second path. len5paths.txt

siddhartha-gadgil commented 3 years ago

For convenience, I am copying the worksheet code which will also run in an ammonite REPL.

import superficial._
import PantsSurface._
import scala.language.postfixOps
val surf = PantsSurface.allClosed(2)(1)
val skewsurf = SkewPantsSurface.enumerate(surf, Vector(0.2), Vector(1, 2))(1)
//val skewsurf = SkewPantsSurface.enumerate(surf, Vector(0,0.2))(1)
val pathsuptolen5 = NormalPath.enumerate[SkewPantsHexagon](skewsurf, Some(5))
pathsuptolen5.size
val clpathsuptolen5 = pathsuptolen5.filter(p => p.isClosed)
clpathsuptolen5.size
val uniqclpathsuptolen5 = NormalPath.uniqRepUptoFlipAndCyclicPer(clpathsuptolen5)
uniqclpathsuptolen5.size
uniqclpathsuptolen5.values.toSet.size
val plpathsuptolen5 = PLPath.enumMinimalClosedFamily(uniqclpathsuptolen5.values.toSet, 0.05, 1.3)
plpathsuptolen5.size
plpathsuptolen5.values.toSet.size
val ndgplpaths = plpathsuptolen5.values.flatten.toVector.filter(_.base.length<5)
ndgplpaths.size
for ( path <- ndgplpaths) { println(PLPath.shorten(skewsurf, path, 0.05, 1.3, uniqclpathsuptolen5, plpathsuptolen5))}
siddhartha-gadgil commented 3 years ago

If you saw the comment just deleted, it did not work. But the error you can see in the stack trace below is caused by looking up a non-existent key in line 538 of PLArc.scala.

error.getStackTrace() 
res36: Array[StackTraceElement] = Array(
  scala.None$.get(Option.scala:627),
  scala.None$.get(Option.scala:626),
  superficial.PLPath$.$anonfun$shorten$5(PLArc.scala:538),
  scala.collection.immutable.Vector1.map(Vector.scala:1872),
  scala.collection.immutable.Vector1.map(Vector.scala:375),
  superficial.PLPath$.shorten(PLArc.scala:537),
  ammonite.$sess.cmd34$.$anonfun$attempt$1(cmd34.sc:1),
  scala.util.Try$.apply(Try.scala:210),
  ammonite.$sess.cmd34$.<clinit>(cmd34.sc:1),
  ammonite.$sess.cmd34.$main(cmd34.sc),
  java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method),
  java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62),
  java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43),
  java.base/java.lang.reflect.Method.invoke(Method.java:566),
  ammonite.runtime.Evaluator$$anon$1.$anonfun$evalMain$1(Evaluator.scala:108),
  ammonite.util.Util$.withContextClassloader(Util.scala:24),
  ammonite.runtime.Evaluator$$anon$1.evalMain(Evaluator.scala:90),
  ammonite.runtime.Evaluator$$anon$1.$anonfun$processLine$2(Evaluator.scala:127),
...
siddhartha-gadgil commented 3 years ago

I reformatted so that the two gets are separated. This shows that the error is that some path looked up while shortening is missing from uniquerepuptoflipandcyclicper:

PLPath(
  NormalPath(
    Vector(
      NormalArc(
        7,
        5,
        SkewPantsHexagon(
          1,
          true,
          Set(
            SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
            SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
            SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
          )
        )
      ),
      NormalArc(
        5,
        4,
        SkewPantsHexagon(
          1,
          false,
          Set(
            SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
            SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
            SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
          )
        )
      ),
      NormalArc(
        7,
        5,
        SkewPantsHexagon(
          1,
          false,
          Set(
            SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
            SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
            SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
          )
        )
      ),
      NormalArc(
        5,
        4,
        SkewPantsHexagon(
          1,
          true,
          Set(
            SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
            SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
            SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
          )
        )
      )
    )
  ),
  Vector(0, 0.0343206395660696, 0.00, 0.0343206395660696),
  Vector(1.55, 0.60, 1.55, 0.60)
)
siddhartha-gadgil commented 3 years ago

Further experiments: we get an error for all 4 PL paths obtained by enumerating the above. Here is one:

NormalPath(
  Vector(
    NormalArc(
      7,
      6,
      SkewPantsHexagon(
        1,
        true,
        Set(
          SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
          SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
          SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
        )
      )
    ),
    NormalArc(
      6,
      5,
      SkewPantsHexagon(
        1,
        false,
        Set(
          SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
          SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
          SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
        )
      )
    ),
    NormalArc(
      5,
      4,
      SkewPantsHexagon(
        1,
        true,
        Set(
          SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
          SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
          SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
        )
      )
    )
  )
)
siddhartha-gadgil commented 3 years ago

error

The above may help. This is a picture of a PL path that could not be shortened. The normal path is in black.

shabarishch commented 3 years ago

Further experiments: we get an error for all 4 PL paths obtained by enumerating the above. Here is one:

NormalPath(
  Vector(
    NormalArc(
      7,
      6,
      SkewPantsHexagon(
        1,
        true,
        Set(
          SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
          SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
          SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
        )
      )
    ),
    NormalArc(
      6,
      5,
      SkewPantsHexagon(
        1,
        false,
        Set(
          SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
          SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
          SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
        )
      )
    ),
    NormalArc(
      5,
      4,
      SkewPantsHexagon(
        1,
        true,
        Set(
          SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1),
          SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1),
          SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2)
        )
      )
    )
  )
)

Thank you for investigating. This path is contractible. The problem is that because of this reason, it is excluded from the enumeration of all closed normalpaths. However, it may come up in the shortening process in the process of shortening a longer path, which explains the key error. I think adding the same check that removes this from enumeration of normalpaths to the function shorten should take care of this problem.

siddhartha-gadgil commented 3 years ago

You do not need to check if you must exclude it anyway (i.e., if it is not a key to uniqrepuptoflipandcyclicper. Instead avoid the get and use flatMap/flatten.

On Mon, 31 May 2021 at 14:19, shabarishch @.***> wrote:

Further experiments: we get an error for all 4 PL paths obtained by enumerating the above. Here is one:

NormalPath( Vector( NormalArc( 7, 6, SkewPantsHexagon( 1, true, Set( SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1), SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1), SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2) ) ) ), NormalArc( 6, 5, SkewPantsHexagon( 1, false, Set( SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1), SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1), SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2) ) ) ), NormalArc( 5, 4, SkewPantsHexagon( 1, true, Set( SkewCurve(PantsBoundary(0, Z3(0)), PantsBoundary(0, Z3(1)), 0.2, 1), SkewCurve(PantsBoundary(0, Z3(2)), PantsBoundary(1, Z3(0)), 0.2, 1), SkewCurve(PantsBoundary(1, Z3(1)), PantsBoundary(1, Z3(2)), 0.2, 2) ) ) ) ) )

Thank you for investigating. This path is contractible. The problem is that because of this reason, it is excluded from the enumeration of all closed normalpaths. However, it may come up in the shortening process in the process of shortening a longer path, which explains the key error. I think adding the same check that removes this from enumeration of normalpaths to the function shorten should take care of this problem.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/siddhartha-gadgil/Superficial/issues/83#issuecomment-851330402, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3K3JB74HP36WDCYAUVK5TTQNEQTANCNFSM452HRFVA .

shabarishch commented 3 years ago

Got it, I will modify the code and then check again.

shabarishch commented 3 years ago

Actually, the issue is more delicate. The above path is excluded from enumeration not because it is contractible, it is because the last few edges are vertex linking and come back to the same face. While writing it (this was done before defining PLPaths), I had felt that since such a normalpath is isotopic to a normalpath with fewer number of arcs (which I felt would clearly have smaller length), we need not enumerate it. But now we do have examples where this does not happen. So the modification should to be made in normalpath enumeration and not the function shorten.

siddhartha-gadgil commented 3 years ago

I have made a change so that if paths are not keys of uniqrepuptoflipandcyclicper then they are omitted.

On Mon, 31 May 2021 at 14:44, shabarishch @.***> wrote:

Actually, the issue is more delicate. The above path is excluded from enumeration not because it is contractible, it is because the last few edges are vertex linking and come back to the same face. While writing it (this was done before defining PLPaths), I had felt that since such a normalpath is isotopic to a normalpath with fewer number of arcs (which I felt would clearly have smaller length), we need not enumerate it. But now we do have examples where this does not happen. So the modification should to be made in normalpath enumeration and not the function shorten.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/siddhartha-gadgil/Superficial/issues/83#issuecomment-851348166, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3K3JFPY4QEXASC7PDI57TTQNHPJANCNFSM452HRFVA .

shabarishch commented 3 years ago

Should I go ahead and modify NormalPath enumeration, then?

siddhartha-gadgil commented 3 years ago

Sure. At the end of the day send a pull request. I will try to optimize performance tomorrow.

siddhartha-gadgil commented 3 years ago

Also pull before you edit since I have made changes.

shabarishch commented 3 years ago

Thank you, I will do so.

siddhartha-gadgil commented 3 years ago

By the way, if at any time you feel this is not an issue (given the change where paths not keys of the map give nothing instead of an error) go ahead and close it.

siddhartha-gadgil commented 3 years ago

Code that runs now, and in reasonable time:

import superficial._
import PantsSurface._
import scala.language.postfixOps
val surf = PantsSurface.allClosed(2)(1)
val skewsurf = SkewPantsSurface.enumerate(surf, Vector(0.2), Vector(1, 2))(1)
val pathsuptolen5 = NormalPath.enumerate[SkewPantsHexagon](skewsurf, Some(5))
pathsuptolen5.size
val clpathsuptolen5 = pathsuptolen5.filter(p => p.isClosed)
clpathsuptolen5.size
val uniqclpathsuptolen5 = NormalPath.uniqueUptoFlipAndCyclicPerm(clpathsuptolen5)
uniqclpathsuptolen5.size
uniqclpathsuptolen5.values.toSet.size
val plpathsuptolen5 = PLPath.enumMinimalClosedFamily(uniqclpathsuptolen5.values.toSet, 0.05, 1.3)
plpathsuptolen5.size
plpathsuptolen5.values.toSet.size
val ndgplpaths = plpathsuptolen5.values.flatten.toVector.filter(_.base.length<5)
ndgplpaths.size
val shortPaths = ndgplpaths.flatMap(path => PLPath.shorten(skewsurf, path, 0.05, 1.3, uniqclpathsuptolen5, plpathsuptolen5))
shortPaths.size