Capture package examples by default #3952
Conversation
d-torrance
force-pushed
the
capture-examples
branch
from
e93f4b4 to
1a6006a
Compare
d-torrance
force-pushed
the
capture-examples
branch
from
1a6006a to
611b66a
Compare
It potentially loads a package and its output isn't captured. As a result, the "no-capture-flag" comment in the "check" example is redundant, so we remove it.
This avoids a couple deprecation warnings in the documentation (which weren't getting captured anyway)
When capturing, we also end up loading Polyhedra, which overrides some methods, resulting in incorrect documentation. For example, net(Polyhedron) doesn't display information about the polyhedron.
Otherwise, the second example will try to create a directory with the same name as the directory from the first example, and we get a warning message in the documentation.
Otherwise, we'll increase the verbosity of subsequently captured examples from this package
Otherwise, we will increase the verbosity in the other examples
Many of the examples call the setNmzOption function, which modifies a global variable that affects the output of other examples.a
It modifies a Python global variable (math.pi) that is used in another example.
This sets a global variable that affects the output of other examples.
d-torrance
force-pushed
the
capture-examples
branch
from
611b66a to
313436e
Compare
There was a problem hiding this comment.
I suspect these are because examples are run by capture in a way that a previous example loading a package has overwritten intersection? Would this be resolved if examples from each package had their own process and all used capture inside that one process? This might be a good idea to do anyway, since the different processes could run in parallel.
There was a problem hiding this comment.
I think that's what we basically do anyway -- start up a new M2 process and run installPackage inside it for each package.
This particular commit isn't really fixing a capture issue, but one I noticed when comparing the capture v. non-capture diffs. We currently have this annoying deprecation warning in the docs (which capture misses), and this change fixes that:
i4 : toricBlowUp(P,Q,1)
Warning: This method is deprecated and will be removed in version 1.11 of Polyhedra. Please consider using polyhedronFromHData instead.
o4 = Polyhedron{...1...}
o4 : Polyhedron
|
Something that's confusing me is the non-captured v. captured diff of some LocalRings examples, e.g.,: --- old/usr/share/doc/Macaulay2/LocalRings/example-output/_local__Resolution.out
+++ new/usr/share/doc/Macaulay2/LocalRings/example-output/_local__Resolution.out
@@ -74,11 +74,11 @@
i11 : C = resolution coker m
1 3 3 1
-o11 = R <-- R <-- R <-- R
-
- 0 1 2 3
+o11 = R <-- R <-- R <-- R <-- 0
+
+ 0 1 2 3 4
-o11 : Complex
+o11 : ChainComplexWithout The package does have OldChainComplexes in |
|
I think the issue is that LocalRings exports Complexes, but calling i1 : locate(res, Module)
o1 = /usr/share/Macaulay2/OldChainComplexes/res.m2:284:37-311:90
o1 : FilePosition
i2 : needsPackage "LocalRings"
-- warning: symbol "GradedModuleMap" in OldChainComplexes.Dictionary is shadowed by a symbol in Complexes.Dictionary
-- use the synonym OldChainComplexes$GradedModuleMap
-- warning: symbol "GradedModule" in OldChainComplexes.Dictionary is shadowed by a symbol in Complexes.Dictionary
-- use the synonym OldChainComplexes$GradedModule
-- warning: symbol "res" in OldChainComplexes.Dictionary is shadowed by a symbol in Complexes.Dictionary
-- use one of the synonyms OldChainComplexes$res, OldChainComplexes$resolution
-- warning: symbol "resolution" in OldChainComplexes.Dictionary is shadowed by a symbol in Complexes.Dictionary
-- use one of the synonyms OldChainComplexes$res, OldChainComplexes$resolution
-- warning: symbol "res" in Complexes.Dictionary is shadowed by a symbol in OldChainComplexes.Dictionary
-- use one of the synonyms Complexes$freeResolution, Complexes$res, Complexes$resolution, freeResolution
-- warning: symbol "GradedModuleMap" in Complexes.Dictionary is shadowed by a symbol in OldChainComplexes.Dictionary
-- use one of the synonyms Complexes$ComplexMap, Complexes$GradedModuleMap, ComplexMap
-- warning: symbol "resolution" in Complexes.Dictionary is shadowed by a symbol in OldChainComplexes.Dictionary
-- use one of the synonyms Complexes$freeResolution, Complexes$res, Complexes$resolution, freeResolution
-- warning: symbol "GradedModule" in Complexes.Dictionary is shadowed by a symbol in OldChainComplexes.Dictionary
-- use one of the synonyms Complex, Complexes$Complex, Complexes$GradedModule
o2 = LocalRings
o2 : Package
i3 : locate(res, Module)
o3 = /usr/share/Macaulay2/Complexes/FreeResolutions.m2:55:36-132:5
o3 : FilePosition
i4 : capture("locate(res, Module)", UserMode => false)
o4 = (false, )
i1 : locate(res, Module)
o1 = /usr/share/Macaulay2/OldChainComplexes/res.m2:284:37-311:90
o1 : FilePosition
i2 :
o4 : SequenceSince I believe we're switching to Complexes in the next release anyway, this is probably a non-issue. |
|
I think this is in pretty good shape now. The diff in the example output seems to be just things like timings, temporary file names, and results of shuffling the order of keys in hash tables: https://gist.github.com/d-torrance/43282670914f1a937e7b6f2faf2d63a0 |
I'm pessimistic about this. Lots of issues remain unsolved and several people I've talked to decided to keep using the old version until those are fixed, which means there may be more undiscovered bugs. |
|
It seems like having a package like LocalRings silently load another package that overrides the behavior of Should Complexes and OldChainComplexes each check the value of |
Well, I think the idea was that the user loading one or the other should replace what |
There was a problem hiding this comment.
I'm not sure what to do with the Complex vs. ChainComplex issue but the rest looks fine.
The build times are impressively shorter. I would like to suggest to single out the install-packages target (in test_build.yml) so that the timing for that is shown separately in every build.
|
The package installation is now in its own step for both the cmake and autotools GitHub builds. |
3 participants
This should significantly speed up building since we won't need to fire up a ton of new M2 processes for each package. This is a draft for now -- we should ensure that the documentation stays unchanged, so I'll compare the build artifacts.