Move Hakaru-owned global symbols to a separate module

Global symbols are currently making our code slightly less maintainable, which, generally, makes the code less and less maintainable as time goes on. The desire to avoid globals has precipitated a discussion which has yielded a potential solution.

We would like to move some global symbols to a separate module, on which all Hakaru modules would then have to depend, directly, or transitively. These symbols would be changed from global symbols to exports of that module; with the appropriate uses statement, very little other code would have to be changed.

This cannot be done with all global symbols; in particular, names like eval/*, depends/*, and print/* must be actual globals, since the Maple global extension mechanism will search for these names directly in the global namespace. Any symbols which are "Hakaru-owned" (interact with, and are defined for, Hakaru and not Maple as a whole - basically, any global name not containing a forward slash) are candidates for such a change; e.g. Bind, Weight, Ret, Msum, Plate, Context, Pair, _Unit, PARTITION, all the primitive measures, all the Hakaru type constructors; all similar such constructors found in Domain.

There are still some details which needs to be discussed:

Precisely which global symbols should actually be moved. The discussion originally arose because some global symbols which had definitions were changed to exports (by cb91d541); the old globals, by virtue of being globals, had to have their definitions added at 'runtime'; whereas exports can have definitions at 'compile-time'. I propose moving all globals which do not have to be globals; that is, avoiding globals as much as possible. If there are any places where global and exported symbols must be treated differently, making them all exports now will make the code more consistent; although I'm not currently aware of any such places.
What rules we would like for symbols exported by such a module; for example, f9bd8215 added definitions to global symbols which do not always produce a term whose 0-th operand is procname; this is valid in those cases (i.e. those symbols are still 'proper constructors') because the alternate return values are part of the Domain interface. Perhaps there are those who would like all global symbols to not have definitions; and that such normalization (as in f9bd8215) should be done by a separate function.
What to name such a module - I propose HakaruGlobals or HakaruGlobalSymbols.
Pertaining to the above two points, perhaps there should be submodules of this module corresponding to each module of Hakaru - each exports symbols related to that module (i.e. Globals:-Hakaru, Globals:-Domain, etc.).

Setting aside symbols that need to be actually global (eval/*, depends/*, print/*) and mutable symbols (I can't think of any in our code that's not local), I understand that a symbol must belong to a module and can either be global and export. From places like https://list.indiana.edu/sympa/arc/ppaml-l/2016-07/msg00107.html and https://github.com/hakaru-dev/hakaru/commit/a0abebb553d6640f2e710f70d7415ce979685b8b#commitcomment-18703930 I learned that whether a symbol that belongs to a module should be global or export should be determined by whether it is assigned a definition: if not then global, if assigned then export. Yes? No?

If most or all globals are placed in a single module, then how about the Hakaru module?

I learned that whether a symbol that belongs to a module should be global or export should be determined by whether it is assigned a definition: if not then global, if assigned then export.

In my understanding, this isn't the distinguishing factor. The differences between global and export symbols which I see are:

to document that a symbol should not have a definition (for exported symbols, I think we can document that fact with regular documentation, i.e. a Maple comment; or by setting <sym> := evaln(`<sym>`))
to ensure a symbols definition is not changed (for globals, we accomplish this by calling protect explicitly; exported symbols are already protected implicitly)
a symbol needs to be used in modules A and B, but neither can depend on the other with use, and for an exported symbol, we don't want to qualify the name with the module prefix every time (perhaps because that is too verbose - but that seems to be the only downside of explicitly qualifying names).

Note that both global and export symbols can have, or not have, a definition:

M1 := module()
  export name0, name1; global name2, name3;
  name1 := proc() 'procname'(args) end proc; 
  name3 := proc() 'procnmae'(args) end proc;
end module:

I believe at some point I incorrectly stated that global symbols could not be assigned from within a module; this is only the case for the module syntax which I have used in Domain (which perhaps indicates I should not be using that syntax); in particular:

M2 := module() global M2_name := proc() 'procname'(args) end proc; end module;

is a parse error.

It seems the only actual issue is with modules which have submodules (which is currently only Domain and NewSLO) and global 'constructors' (e.g. Domain:-DConstrain, NewSLO:-Integrand); in those submodules, I don't see any choice but to make the symbol global; or export the symbol from an entirely separate module. The problem there is that a submodule cannot use its supermodule (fairly obvious), or even another submodule (not so obvious! - or, I just haven't figured out how yet).

If a particular constructor needs a definition (currently only in Domain), it can still be global, protected in ModuleLoad, and given a definition in the body of the module (the last point requires changing the syntax of Domain; but that is purely a syntactic change). The other choice is to export the constructor (and it's definition) from e.g. Hakaru and then use Hakaru in the modules which need that constructor.

If most or all globals are placed in a single module, then how about the Hakaru module?

This is probably the simplest choice.

Yes, I think that we should explicitly put all our global symbols in one place. That one place should be 'minimal', so that it can be loaded first. If Hakaru is that module, that's good. It might need a little tweaking to get there?

Trying to change global symbols in Hakaru to exports produces the following error:

Error, (in unknown) attempting to assign to `Bound` which is protected.  Try declaring `local Bound`; see ?protect for details.

which has to do with the fact that both Domain and Hakaru would then export a symbol called Bound, and Domain uses Hakaru. A minimal version of this issue is as follows:

M1 := module() option package; export Foo; end module:
M2 := module() uses M1; export Foo := "Foo"; end module:

(NB: Do modules actually not have their own namespaces, or is this a bug?)

As it seems that this problem is very much localized to Domain, I've decided to restrict the scope of this issue to Domain, and leave the rest of Hakaru alone for now. This in a way makes sense; the issue was originally precipitated by a discussion about Domain in particular.

I've moved the Domain constructors to a separate (top-level) module, from which they are exported. It probably is not sensible to move them to Hakaru, since they are really local to Domain (but cannot be local, as with KB:-Introduce et al., since Domain has submodules).

I'll close this for now (via a2ab2866) - feel free to reopen if you think more symbols need to be moved (and have an idea as to how to deal with the strange namespace error above!)

I just tried make NewSLOTests.out under 4753a2815e51ff8ee783800b721cbc919b667794 and got lots of errors. Is it just me? It seems that uses Domain_Type needs to be added in more places? And perhaps the Domain module should no longer say global DSplit; global DInto; global DNoSol;?

I just tried make NewSLOTests.out under 4753a28 and got lots of errors.

Could you include (one of) the errors? I can't reproduce this.

And perhaps the Domain module should no longer say global DSplit; global DInto; global DNoSol;?

Indeed - this is fixed now (35825171d8c). But the test passed for me both with and without the removal of this; presumably because exported symbols take precedence over global symbols.

Could you include (one of) the errors? I can't reproduce this. Using 04c28e897e655536ba31618e75fa4f68c022aff9:

$ maple update-archive.mpl # no errors
$ make NewSLOTests.out
maple -q NewSLOTests.mpl 2>&1 | tee NewSLOTests.out | (grep -v " passed$" || true)
slice sampling failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DNoSol("invalid input: SolveTools:-SemiAlgebraic expects its 1st argument, osys, to be of type {list({ratpoly(rational), ratpoly(rational) = ratpoly(rational), ratpoly(rational) <> ratpoly(rational), ratpoly(rational) <= ratpoly(rational), ratpoly(rational) < ratpoly(rational)}), set({ratpoly(rational), ratpoly(rational) = ratpoly(rational), ratpoly(rational) <> ratpoly(rational), ratpoly(rational) <= ratpoly(rational), ratpoly(rational) < ratpoly(rational)})}, but received {0 < lu, 0 < exp(-lu), `x乕` < 2^(1/2)*lu^(1/2), -2^(1/2)*lu^(1/2) < `x乕`}")]
Error, (in CodeTools:-Test) TEST FAILED: slice sampling
Error, (in unknown) invalid input: cmp_simp_sh expects its 4th argument, sh, to
be of type {DomNoSol, DomShape}, but received DNoSol("invalid input:
SolveTools:-SemiAlgebraic expects its 1st argument, osys, to be of type
{list({ratpoly(rational), ratpoly(rational) = ratpoly(rational),
ratpoly(rational) <> ratpoly(rational), ratpoly(rational) <= ratpoly(rational),
ratpoly(rational) < ratpoly(rational)}), set({ratpoly(rational), ratpoly(rati...
fake piecewise 1 failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum()]
Error, (in CodeTools:-Test) TEST FAILED: fake piecewise 1
fake piecewise 2 failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum()]
Error, (in CodeTools:-Test) TEST FAILED: fake piecewise 2
t9 failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(3 < `a0佭`,`a0佭` < 7))]
Error, (in CodeTools:-Test) TEST FAILED: t9
t9a failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(3 < `a0佱`,`a0佱` < 7))]
Error, (in CodeTools:-Test) TEST FAILED: t9a
Warning, siderels bug:
'when calling '`simplify/siderels`:-`simplify/siderels`'. Received: 'side
relations must be polynomials in (name or function) variables''
when calling coulditbe(0 < y) assuming ([y <
1/2*2^(1/2)/Pi^(1/2)*exp(-1/2*x^2)])
Warning, siderels bug:
'when calling '`simplify/siderels`:-`simplify/siderels`'. Received: 'side
relations must be polynomials in (name or function) variables''
when calling coulditbe(0 < y) assuming ([y <
1/2*2^(1/2)/Pi^(1/2)*exp(-1/2*`x侏`^2)])
clamp condition to move it out of integral failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DNoSol("invalid input: SolveTools:-SemiAlgebraic expects its 1st argument, osys, to be of type {list({ratpoly(rational), ratpoly(rational) = ratpoly(rational), ratpoly(rational) <> ratpoly(rational), ratpoly(rational) <= ratpoly(rational), ratpoly(rational) < ratpoly(rational)}), set({ratpoly(rational), ratpoly(rational) = ratpoly(rational), ratpoly(rational) <> ratpoly(rational), ratpoly(rational) <= ratpoly(rational), ratpoly(rational) < ratpoly(rational)})}, but received {0 < y, `x侏` < (-ln(2)-ln(Pi)-2*ln(y))^(1/2), y < 1/2/Pi^(1/2)*2^(1/2), -(-ln(2)-ln(Pi)-2*ln(y))^(1/2) < `x侏`}")]
Error, (in CodeTools:-Test) TEST FAILED: clamp condition to move it out of
integral
clamp condition to move it out of integral (ln coordinate) failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DNoSol("invalid input: SolveTools:-SemiAlgebraic expects its 1st argument, osys, to be of type {list({ratpoly(rational), ratpoly(rational) = ratpoly(rational), ratpoly(rational) <> ratpoly(rational), ratpoly(rational) <= ratpoly(rational), ratpoly(rational) < ratpoly(rational)}), set({ratpoly(rational), ratpoly(rational) = ratpoly(rational), ratpoly(rational) <> ratpoly(rational), ratpoly(rational) <= ratpoly(rational), ratpoly(rational) < ratpoly(rational)})}, but received {ly < -1/2*ln(2)-1/2*ln(Pi), `x侓` < (-ln(2)-ln(Pi)-2*ly)^(1/2), -(-ln(2)-ln(Pi)-2*ly)^(1/2) < `x侓`}")]
Error, (in CodeTools:-Test) TEST FAILED: clamp condition to move it out of
integral (ln coordinate)
minor miracle failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(0 < `y侗`,`y侗` < 1))]
Error, (in CodeTools:-Test) TEST FAILED: minor miracle
exponentiated indicator failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(`x供` <= 1/2,0 < `x供`))]
Error, (in CodeTools:-Test) TEST FAILED: exponentiated indicator
negated and conjoined indicator failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(0 <= `x侟`,`x侟` < 1))]
Error, (in CodeTools:-Test) TEST FAILED: negated and conjoined indicator
bounds ordering failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum()]
Error, (in CodeTools:-Test) TEST FAILED: bounds ordering
simplify under sum failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(`x侧` <= 1/2,0 < `x侧`))]
Error, (in CodeTools:-Test) TEST FAILED: simplify under sum
simplify under piecewise failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSplit(PARTITION([Piece(c <= 0,DSum()), Piece(0 < c,DSum(DConstrain(`x侭` <= 1/2,0 < `x侭`)))]))]
Error, (in CodeTools:-Test) TEST FAILED: simplify under piecewise
banish piecewise 6 failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSplit(PARTITION([Piece(`x俻` < 0,DSum()), Piece(`x俻` = 0,DSum(DConstrain(0 < `xx俵`,`xx俵` < 1))), Piece(0 < `x俻`,DSum(DConstrain(0 < `xx俵`,`xx俵` < 1)))]))]
Error, (in CodeTools:-Test) TEST FAILED: banish piecewise 6
banish piecewise 7 failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSplit(PARTITION([Piece(`x倇` <= 0,DSum(DConstrain(0 < `xx倁`,`xx倁` < 1))), Piece(0 < `x倇`,DSum())]))]
Error, (in CodeTools:-Test) TEST FAILED: banish piecewise 7
Test failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(0 < `xx個`,`xx個` < 1))]
Error, (in CodeTools:-Test) TEST FAILED
Tests.RoundTrip.rmProg1 failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(3 < `a4偯`,`a4偯` < 8,1 < `a5偭`,`a5偭` < 4))]
Error, (in CodeTools:-Test) TEST FAILED: Tests.RoundTrip.rmProg1
roundtrip despite banishing failure failed
Received error: [unknown, "invalid input: %1 expects its %-2 argument, %3, to be of type %4, but received %5", Domain:-Improve:-cmp_simp_sh, 4, sh, {DomNoSol, DomShape}, DSum(DConstrain(0 <= `xx傣`,`xx傣` < 1))]
Error, (in CodeTools:-Test) TEST FAILED: roundtrip despite banishing failure

This is indeed different than what I get; I'm not sure how to trigger the above (can you try deleting the archive first? Maybe it's an issue with that).

Does 94bd8360d873 help?

As of 0a68a289ea8749141b6deeaa3f8425aa493d8f79, I get fewer messages:

Warning, siderels bug:
    'when calling '`simplify/siderels`:-`simplify/siderels`'. Received: 'side
relations must be polynomials in (name or function) variables''
when calling coulditbe(0 < y) assuming ([y <
1/2*2^(1/2)/Pi^(1/2)*exp(-1/2*x^2)])
Warning, siderels bug:
    'when calling '`simplify/siderels`:-`simplify/siderels`'. Received: 'side
relations must be polynomials in (name or function) variables''
when calling coulditbe(0 < y) assuming ([y <
1/2*2^(1/2)/Pi^(1/2)*exp(-1/2*`x価`^2)])

Those are just warnings (which maybe should be produced with userinfo and not WARNING, in which case you wouldn't always have to see them). I'm hoping this will one day be fixed (but it isn't related, I think).

Ok, I guess something fixed it on my machine...

hakaru-dev / hakaru

Move Hakaru-owned global symbols to a separate module #65