circular dependency loops among packages should be flagged as errors

[DanGrayson]

I just did a test and got a surprising result -- these two packages give no error, even back to M2 1.12:

newPackage ( "Foo",
     PackageImports => { "Bar" }
     )

-----------------------------------------------------------------------------

newPackage ( "Bar",
     PackageImports => { "Foo" }
     )

Will fix.

Originally posted by @DanGrayson in https://github.com/Macaulay2/M2/issues/1950#issuecomment-1011247096

[mahrud]

Maybe what you want is another option like PackageDependencies because it makes more sense to me to have PackageImports simply mean the dictionary of that package must be loaded when the code runs, not when it is loaded. From this perspective, there is no problem with circular imports unless loading a package runs some code (which I think should be avoided anyway).

Here is a simple example:

i1 : f = x -> g(x);

i2 : g = x -> 2*x;

i3 : f(2)

o3 = 4

M2 doesn't require g to be defined when f is defined. So why can't I do the same with packages? That is, load whichever package the user wants loaded, then load the PackageImports.

[DanGrayson]

In your example, the line i1 will create a symbol called g in the user's dictionary, and the code of f will refer to that symbol. Then the line i2 will assign that same symbol a value. For packages, there's no way for a package to know at load time which package g is supposed to come from.

[mahrud]

Most often packages use methods rather than functions, so it shouldn't be an issue most of the time. Even if a package doesn't know at load time which package declares a method, as long as the types are defined, it should still be okay. Making changes to allow this is a more productive change than to disallow it. Like I said before, your suggested change would make designing package groups harder.

[mikestillman]

Actually, it will be a problem whenever one package creates a Type, and the other uses it, isn't that right? This is a common situation, in any case.

[mahrud]

A simple improvement would be to make the exports list of a package a part of newPackage, so simply reading the prelude (e.g. a call to readPackage) would provide all necessary information about which package the symbols belong to. Then loading the rest of the package can occur later.

Another way might be to make a "stack" of packages to be loaded, then load the packages in reverse order until and including the first call to exports, then defer loading the rest of the package and continue loading the exports of all packages in the stack, then load the rest of all the packages in any order.

[DanGrayson]

That's too complicated to explain to new users.

[mahrud]

That's too complicated to explain to new users.

The symbols and symbol bodies and frames is too complicated for me, and I'm not a new user. Does that mean it should go? If so then we can make every single symbol global, then the problem you described above would also go away and we can have circular imports :)

[DanGrayson]

I think it's good that each package has its own little sandbox for its symbols.

[mahrud]

@mikestillman, with Dan's new changes in #2381, I don't know of any way to make Truncations, Complexes, and NormalToricVarieties work together anymore. We had talked about using hooks, but that slows down truncations to a point that I don't think is acceptable. I think the two of you should figure out a new way to manage symbol ownership that doesn't make circular dependencies impossible.

The new changes also break the package group prototype that I was testing for a while (see https://github.com/Macaulay2/M2/issues/1950#issuecomment-1007677672).

[mikestillman]

@mikestillman, with Dan's new changes in #2381, I don't know of any way to make Truncations, Complexes, and NormalToricVarieties work together anymore. We had talked about using hooks, but that slows down truncations to a point that I don't think is acceptable. I think the two of you should figure out a new way to manage symbol ownership that doesn't make circular dependencies impossible.

The new changes also break the package group prototype that I was testing for a while (see #1950 (comment)).

@mahrud What is specifically the issue? (i.e. which symbols are causing problems). For this particular set of packages, I was thinking that Complexes would not define a truncation routine, but Truncations would. What is the specific problem with NormalToricVarieties and Truncations? (I'll look more carefully too).

Still, I agree that there are issues with ownership of symbols, especially if we want several packages to work together. I'll discuss this with @DanGrayson

[mahrud]

What is specifically the issue? (i.e. which symbols are causing problems). For this particular set of packages, I was thinking that Complexes would not define a truncation routine, but Truncations would. What is the specific problem with NormalToricVarieties and Truncations? (I'll look more carefully too).

There are routines in NormalToricVarieties I'd like to improve (like cohomology) that require Truncations, but Truncations requires NormalToricVarieties.

I also have a branch where I've turned m2/varieties.m2 into a package, which can then be loaded by other packages like NormalToricVarieties that extend varieties and sheaves, but this package depends on Truncations, which as I said required NormalToricVarieties.

It seems to me like packages owning symbols is more of a hassle than a feature from the point of view of doing math.

[DanGrayson]

fixed:

i2 : loadPackage "Foo"
Foo.m2:1:1:(3):[38]: error: package Foo not reloaded; try Reload => true
Bar.m2:2:1:(3):[23]: --back trace--
Foo.m2:1:1:(3):[8]: --back trace--

[mikestillman]

I think that this isn't quite solved. We seem to have cases where symbols owned by packages is making it impossible to load (both) packages? @mahrud Could you be a bit more specific on the example issue you had?

[mahrud]

I think that this isn't quite solved. We seem to have cases where symbols owned by packages is making it impossible to load (both) packages? @mahrud Could you be a bit more specific on the example issue you had?

Sure. I'm a bit busy this week but I can provide a code snipped next week.

Here is a toy version: both rank and dim are defined in the Core. Suppose I have two types T1 and T2 defined by packages P1 and P2, respectively, and let's say rank T1 depends on rank T2 and dim T2 depends on dim T1.

This is currently not possible to do currently (without using various kludges, like calling needsPackage inside a function, which will slow down the code if a rank or dimension needs to be computed many times) unless the packages are combined.

[DanGrayson]

I don't see a principled way to make that work.

I'm actually glad that this issue is still open, because I have a better fix coming along on my branch fix-documentation-of-items-from-packages -- the error message will look like this:

i1 : loadPackage "Foo"
Bar.m2:1:1:(3):[23]: -- error: package Foo currently being loaded, and 'needsPackage' has been called on it
Foo.m2:1:1:(3):[8]: --back trace--