HPC-GAP specific code for QuaternionAlgebraData

[ThomasBreuer]

Just for curiosity:

I stumbled over HPCGAP specific code for dealing with the cache QuaternionAlgebraData,
see lib/algsc.gi.
The function StoreQuaternionAlgebraData adds the algebra and related data to this cache if necessary.
In the case that no new entry is needed, this function creates a new algebra,
which is equal (in the sense of \=) to the given one, and returns it.

Why is this new object necessary, and why would it be wrong to keep (in the function QuaternionAlgebra)
the given algebra instead of replacing it by the one returned by StoreQuaternionAlgebraData?

[olexandr-konovalov]

To help to quickly see the code:

gap/lib/algsc.gi

Lines 689 to 711 in 30c038b

	#V QuaternionAlgebraData
	##
	InstallFlushableValue( QuaternionAlgebraData, [] );
	if IsHPCGAP then
	ShareSpecialObj( QuaternionAlgebraData );
	BindGlobal("StoreQuaternionAlgebraData", function( a, b, F, A )
	local stored;
	atomic readwrite QuaternionAlgebraData do
	stored:= First( QuaternionAlgebraData,
	t -> t[1] = a and t[2] = b and
	IsIdenticalObj( t[3], FamilyObj( F ) ) );
	if stored = fail then
	# Store the data for the next call.
	Add( QuaternionAlgebraData, MakeImmutable([ a, b, FamilyObj( F ), A ]) );
	else
	A:= AlgebraWithOne( F, GeneratorsOfAlgebra( stored[4] ), "basis" );
	SetGeneratorsOfAlgebra( A, GeneratorsOfAlgebraWithOne( A ) );
	fi;
	od;
	return A;
	end);
	fi;

[fingolfin]

@ThomasBreuer That's an interesting question, and I wondered the same -- but it actually has nothing to do with HPC-GAP; for HPC-GAP, we only tried to carefully replicate the existing behavior of QuaternionAlgebra, which always did it this way. From GAP 4.8:

...
    # Generators in the right family may be already available.
    stored:= First( QuaternionAlgebraData,
                    t ->     t[1] = a and t[2] = b
                         and IsIdenticalObj( t[3], FamilyObj( F ) ) );
    if stored <> fail then
      A:= AlgebraWithOne( F, GeneratorsOfAlgebra( stored[4] ), "basis" );
      SetGeneratorsOfAlgebra( A, GeneratorsOfAlgebraWithOne( A ) );
    else
...
      A:= AlgebraByStructureConstantsArg(
...
      # Store the data for the next call.
      Add( QuaternionAlgebraData, [ a, b, FamilyObj( F ), A ] );

    fi;
...
    return A;

My guess is that the function is supposed to return a fresh algebra every time, without stored knowledge about its properties. Whether that's desirable or not is another question, but it certainly matches what other group/algebra/... constructors do.

[ThomasBreuer]

@fingolfin Thanks for your reply.

The idea behind this caching mechanism is that QuaternionAlgebra may reuse the infrastructure
(the multiplication table, and the family of objects)
but does not return the cached algebra since the domain of coefficients can be different from the desired one.
One could think of improving this, either by comparing the coefficients domains or by changing the caching mechanism such that only the lists of generators are stored.

I think that now I understand the code better.
Suppose we are in the IsHPCGAP case.
In the first call to QuaternionAlgebra, the cache is empty, and we call AlgebraByStructureConstantsArg.
Then StoreQuaternionAlgebraData extends the cache and returns the given algebra, which is fine.
In further calls to QuaternionAlgebra, we find a stored algebra in the cache,
and do not call StoreQuaternionAlgebraData, which is also fine.
The code deals with a third situation (and I think that this situation is HPCGAP specific).
Namely, checking the cache may be without success, and thus one wants to extend the cache.
However, it turns out that meanwhile the cache contains the entry in question,
because another thread has been asked for the algebra in question.
And now the "new" algebra gets replaced by the stored one,
in order to make sure that there aren't two incompatible versions around in the session.

Thus the handling of objects is logically correct.
(I think the code would be easier to understand if we had StoreQuaternionAlgebraData and
FetchQuaternionAlgebraData also in the non-HPCGAP case, for handling the cache,
and would distinguish between HPCGAP and non-HPCGAP only inside these functions.)

[fingolfin]

Sure, StoreQuaternionAlgebraData and FetchQuaternionAlgebraData could be made available and used in the non-HPC-GAP case, too, for more uniform code. Back when I added them, I simply wanted the diff to be minimal, which is why I didn't do that. But I see no reason against doing it now.

[ThomasBreuer]

When I find some time, I will prepare a pull request for that.