Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CreatePolymakeObject(); fails the second time when using GAP workspace files #17

Open
kiryph opened this issue Jan 23, 2023 · 2 comments

Comments

@kiryph
Copy link

kiryph commented Jan 23, 2023

Background

GAP recommends to use workspace files for ease of use of GAP:

The recommended way to start GAP is to load an existing workspace file, because this reduces the startup time of GAP drastically. So if you have installed GAP yourself then you should think about creating a workspace file immediately after you have started GAP, and then using this workspace file later on, whenever you start GAP.
https://docs.gap-system.org/doc/ref/chap3_mj.html#X7CB282757ACB1C09

Hence, my gaprc contains following line:

SaveOnExitFile := "/Users/kiryph/.cache/gap/workspacefile";

Issue

However, I have encountered an error when using polymaking.

Start gap without workspace file:

$ \gap
 ┌───────┐   GAP 4.12.1 of 2022-10-20
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-apple-darwin21-default64-kv8
 Configuration:  gmp 6.2.1, GASMAN, readline
 Loading the library and packages ...
 Packages:   AClib 1.3.2, Alnuth 3.2.1, AtlasRep 2.1.6, AutoDoc 2022.10.20, AutPGrp 1.11, Browse 1.8.18, CaratInterface 2.3.4,
             Congruence 1.2.4, CRISP 1.4.5, crypting 0.10.3, Cryst 4.1.25, CrystCat 1.1.10, CTblLib 1.3.4, curlInterface 2.3.1,
             datastructures 0.2.7, Digraphs 1.6.0, EDIM 1.3.6, FactInt 1.6.3, ferret 1.0.9, FGA 1.4.0, Forms 1.2.9, FUtil 0.1.5,
             GAPDoc 1.6.6, genss 1.6.8, GRAPE 4.8.5, groupoids 1.71, HAP 1.47, HAPcryst 0.1.15, images 1.3.1, IO 4.8.0,
             IRREDSOL 1.4.3, json 2.1.1, LAGUNA 3.9.5, Memoisation 1.0, nq 2.5.8, orb 4.9.0, Polenta 1.3.10, Polycyclic 2.16,
             polymaking 0.8.6, PrimGrp 3.4.2, RadiRoot 2.9, recog 1.4.2, ResClasses 4.7.3, Semigroups 5.0.2, singular 2022.09.23,
             SmallGrp 1.5, Sophus 1.27, SpinSym 1.5.2, TomLib 1.2.9, TransGrp 3.6.3, utils 0.77, XMod 2.88
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
gap> permutahedron:=CreatePolymakeObject();
<polymake object. No properties known>
gap> <C-d>

Start gap with workspace (I have a shell alias for this called simply gap)

$ \gap -b -L /Users/kiryph/.cache/gap/workspacefile
gap> permutahedron:=CreatePolymakeObject();
Error, PrintTo: cannot open '/var/folders/2z/bljkcy1j2cbf7l9zf3dk0nqc0000gn/T//gaptempdirGcUUsd/poly368' for output in
  CallFuncList( PRINT_TO, arg ); at /usr/local/Cellar/gap/4.12.1/libexec/lib/streams.gd:1070 called from
PrintTo( name, "" ); at /usr/local/Cellar/gap/4.12.1/libexec/pkg/polymaking/lib/construct.gi:28 called from
CreateEmptyFile( filename ); at /usr/local/Cellar/gap/4.12.1/libexec/pkg/polymaking/lib/construct.gi:59 called from
CreatePolymakeObjectFromFile( dir, newname ) at /usr/local/Cellar/gap/4.12.1/libexec/pkg/polymaking/lib/construct.gi:79 called from
CreatePolymakeObject( "poly", dir ) at /usr/local/Cellar/gap/4.12.1/libexec/pkg/polymaking/lib/construct.gi:97 called from
CreatePolymakeObject( POLYMAKE_DATA_DIR ) at /usr/local/Cellar/gap/4.12.1/libexec/pkg/polymaking/lib/construct.gi:115 called from
...  at *stdin*:1

As far as I understand it:

When polymaking is loaded and POLYMAKE_DATA_DIR is not yet set, a temporary directory is created and assigned to POLYMAKE_DATA_DIR.

> SetPolymakeDataDirectory( dir )
Sets the directory in which all polymake files are created to dir. The standard place for these files is a temporary directory generated when the package is loaded. This manipulates POLYMAKE_DATA_DIR (3.2-2).
https://gap-packages.github.io/polymaking/doc/chap1.html#X851C596486F918F0

When starting gap with the workspace file, POLYMAKE_DATA_DIR is restored and points to a no longer existing directory what polymake expects to exist:

If called without arguments, this method generates an empty file in the directory defined by POLYMAKE_DATA_DIR (3.2-2). If a directory dir is given (this directory must exist), an empty file is generated in this directory.

Possible solutions

  • Add SetPolymakeDataDirectory( Directory("persistent_dir") ); in gaprc
  • polymaking creates a missing POLYMAKE_DATA_DIR

I feel like polymaking should be able to work with temporary directories: temporary files pile up in a persistent directory over time.

However, I am not sure when using a GAP workspace file and I want to be able to continue using polymake objects where I left the last time gap that the files also must be persistent.

If this is the case, can the documentation mention the recommended way when using workspace files?

@kiryph kiryph changed the title CreatePolymakeObject(); fails the second when using GAP workspace files CreatePolymakeObject(); fails the second time when using GAP workspace files Jan 23, 2023
@fingolfin
Copy link
Member

The fact that polymaking creates a temporary directory, references it in POLYMAKE_DATA_DIR and expects it to always be around, is certainly a bug, even without using workspaces.

However, just re-creating the dir also doesn't sound great to me (though it might be the quickest temporary workaround).

An ideal solution would be to revise the whole thing:

  • SetPolymakeDataDirectory should be deprecated, in favor of a GAP UserPreference (and similar for SetPolymakeCommand)
    • this requires among other things suitable DeclareUserPreference invocations in polymaking
  • there are two places which currently read the value of POLYMAKE_DATA_DIR:
    lib/construct.gi:49:    return CreatePolymakeObjectFromFile(POLYMAKE_DATA_DIR, name);
    lib/construct.gi:115:    return CreatePolymakeObject(POLYMAKE_DATA_DIR);
    
    These should instead call a new helper which does this: if the userprefence is set to a user path, use that; otherwise use POLYMAKE_DATA_DIR
  • finally, CallAndInstallPostRestore should be used wrap the code which sets the initial value for POLYMAKE_DATA_DIR, ensuring that it is updated after reloading a workspace.

The reason we can't just do this final step in isolation is that then we cannot distinguish the case were the user specific a custom location from the case were we asked the operating system for a temporary directory... Though perhaps this could be achieved by simpler means for now, e.g.: have a second variable POLYMAKE_DATA_DIR_IS_TEMP set to true after the line POLYMAKE_DATA_DIR:=DirectoryTemporary();, and to false from inside SetPolymakeDataDirectory... Then one could do something like this (untested):

CallAndInstallPostRestore( function()
  if not IsBound(POLYMAKE_DATA_DIR) then
    BindGlobal("POLYMAKE_DATA_DIR", DirectoryTemporary());
    POLYMAKE_DATA_DIR_IS_TEMP := true;
  elif IsBound(POLYMAKE_DATA_DIR_IS_TEMP) and ValueGlobal("POLYMAKE_DATA_DIR_IS_TEMP") = true then
    MakeReadWriteGlobal("POLYMAKE_DATA_DIR");
    UnbindGlobal("POLYMAKE_DATA_DIR");
    BindGlobal("POLYMAKE_DATA_DIR", DirectoryTemporary());
  else
    # TODO: possibly re-create POLYMAKE_DATA_DIR directory??
  fi;
end );

@kiryph
Copy link
Author

kiryph commented Nov 24, 2023

I have deleted my unrelated comments about a segmentation fault.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants