siddhartha-gadgil / Superficial

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

Malformed PLArc while enumerating #88

Closed siddhartha-gadgil closed 3 years ago

siddhartha-gadgil commented 3 years ago

When enumerating, we get an error because a PLArc does not satisfy its initialization requirement. This is a new error presumably introduced by recent corrections.

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))
val clpathsuptolen5 = pathsuptolen5.filter(p => p.isClosed)
val uniqclpathsuptolen5 = NormalPath.uniqueUptoFlipAndCyclicPerm(clpathsuptolen5)
val plpathsuptolen5 = PLPath.enumMinimalClosedFamily(uniqclpathsuptolen5.values.toVector, 0.05, 1.3)

The error seen:

java.lang.IllegalArgumentException: requirement failed
  scala.Predef$.require(Predef.scala:325)
  superficial.PLPath.<init>(PLArc.scala:261)
  superficial.PLPath$.$anonfun$appendPLArc$6(PLArc.scala:392)
  scala.collection.Iterator$$anon$9.next(Iterator.scala:575)
  scala.collection.mutable.Growable.addAll(Growable.scala:65)
  scala.collection.mutable.Growable.addAll$(Growable.scala:60)
  scala.collection.immutable.VectorBuilder.addAll(Vector.scala:1565)
  scala.collection.immutable.Vector$.from(Vector.scala:1349)
  scala.collection.immutable.Vector$.from(Vector.scala:34)
  scala.collection.SeqFactory$Delegate.from(Factory.scala:306)
  scala.collection.immutable.IndexedSeq$.from(Seq.scala:115)
  scala.collection.immutable.IndexedSeq$.from(Seq.scala:112)
  scala.collection.IterableOps$WithFilter.map(Iterable.scala:884)
  superficial.PLPath$.$anonfun$appendPLArc$4(PLArc.scala:380)
  scala.collection.parallel.AugmentedIterableIterator.flatmap2combiner(RemainsIterator.scala:123)
  scala.collection.parallel.AugmentedIterableIterator.flatmap2combiner$(RemainsIterator.scala:121)
  scala.collection.parallel.immutable.ParHashSet$ParHashSetIterator.flatmap2combiner(ParHashSet.scala:77)
  scala.collection.parallel.ParIterableLike$FlatMap.leaf(ParIterableLike.scala:1024)
  scala.collection.parallel.Task.$anonfun$tryLeaf$1(Tasks.scala:52)
  scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
  scala.util.control.Breaks$$anon$1.catchBreak(Breaks.scala:97)
  scala.collection.parallel.Task.tryLeaf(Tasks.scala:55)
  scala.collection.parallel.Task.tryLeaf$(Tasks.scala:49)
  scala.collection.parallel.ParIterableLike$FlatMap.tryLeaf(ParIterableLike.scala:1020)
  scala.collection.parallel.AdaptiveWorkStealingTasks$AWSTWrappedTask.internal(Tasks.scala:159)
  scala.collection.parallel.AdaptiveWorkStealingTasks$AWSTWrappedTask.internal$(Tasks.scala:156)
  scala.collection.parallel.AdaptiveWorkStealingForkJoinTasks$AWSFJTWrappedTask.internal(Tasks.scala:304)
  scala.collection.parallel.AdaptiveWorkStealingTasks$AWSTWrappedTask.compute(Tasks.scala:149)
  scala.collection.parallel.AdaptiveWorkStealingTasks$AWSTWrappedTask.compute$(Tasks.scala:148)
  scala.collection.parallel.AdaptiveWorkStealingForkJoinTasks$AWSFJTWrappedTask.compute(Tasks.scala:304)
  java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189)
  java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
  java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
  java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
  java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
  java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)
shabarishch commented 3 years ago

I will investigate this.

siddhartha-gadgil commented 3 years ago

Confirmed that this was caused by commit 16a4404456d2f6836aebadb6e1af9b ("Corrections" - after reverting this the bug is not there. The branch plarc-requirement has a version without this bug.

siddhartha-gadgil commented 3 years ago

The error in this case is caused by the switch of + 1 and + 2 in lines 755, 756 of PantsSurface. This change was presumably made to correct some other case. The coditions may need to be refined.

shabarishch commented 3 years ago

I have fixed the error.

shabarishch commented 3 years ago

The earlier fix that resulted in the aforementioned commit was incorrect. Now I have made the correct fix.