[FTheoryTools] Reduce allocs

[HereAround]

For my convenience, based on #4446.

cc @apturner @emikelsons

[fingolfin]

Just out of curiosity, why Non-Abelian and not the more usual non-abelian?

[HereAround]

As a humble PhD student, @mohamed-barakat once showed me his (at the time) writing style for proposals, in which the capitalized the name of Mathematicians to honor their contributions. In the case at hand, Abel. I believe this style stayed with me since, although I am sure to have been inconsistent.

On second look, I favor non-abelian here. More consistent. Will update this shortly.

[fingolfin]

The way I learned this is different: names and words derived from thsoe are capitalized. So it is e.g. "Noetherian ring" or "Riemannian manifold" or "Coxeter group". But "abelian" is a rare exception, the name is so ingrained that it is no longer considered a name but it really turned into a regular word. In a sense that's the greater honor :-)

[fingolfin]

Indeed I just found this on Wikipedia:

Among mathematical adjectives derived from the proper name of a mathematician, the word "abelian" is rare in that it is often spelled with a lowercase a, rather than an uppercase A, the lack of capitalization being a tacit acknowledgment not only of the degree to which Abel's name has been institutionalized but also of how ubiquitous in modern mathematics are the concepts introduced by him

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.39%. Comparing base (d589f51) to head (5b54c55).
Report is 43 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4453   +/-   ##
=======================================
  Coverage   84.39%   84.39%           
=======================================
  Files         668      668           
  Lines       88567    88560    -7     
=======================================
- Hits        74746    74742    -4     
+ Misses      13821    13818    -3     

Files with missing lines	Coverage Δ
...al/FTheoryTools/src/G4Fluxes/special_attributes.jl	78.40% <100.00%> (+0.33%)	⬆️
...rimental/FTheoryTools/src/TateModels/attributes.jl	86.58% <ø> (ø)
...l/FTheoryTools/src/WeierstrassModels/attributes.jl	85.71% <100.00%> (-0.83%)	⬇️
experimental/FTheoryTools/test/tate_models.jl	100.00% <ø> (ø)

... and 1 file with indirect coverage changes

[HereAround]

@apturner and @emikelsons I have worked over this code a little, in that I keep the original code for singular_loci as a comment. Other than that - no changes. Ready for your reviews.

[fingolfin]

Just to say (and feel free to ignore this) you can also write this

Suggested change

	f_primes = factor(weierstrass_section_f(w))
	g_primes = factor(weierstrass_section_g(w))
	f_prime_dict = Dict(fp[1] => fp[2] for fp in f_primes)
	g_prime_dict = Dict(gp[1] => gp[2] for gp in g_primes)
	f_primes = factor(weierstrass_section_f(w))
	g_primes = factor(weierstrass_section_g(w))
	f_prime_dict = Dict(p => n for (p,n) in f_primes)
	g_prime_dict = Dict(p => n for (p,n) in g_primes)

or even this

Suggested change

	f_primes = factor(weierstrass_section_f(w))
	g_primes = factor(weierstrass_section_g(w))
	f_prime_dict = Dict(fp[1] => fp[2] for fp in f_primes)
	g_prime_dict = Dict(gp[1] => gp[2] for gp in g_primes)
	f_prime_dict = Dict(p => n for (p,n) in factor(weierstrass_section_f(w)))
	g_prime_dict = Dict(p => n for (p,n) in factor(weierstrass_section_g(w)))

Of course even simpler is

Suggested change

	f_primes = factor(weierstrass_section_f(w))
	g_primes = factor(weierstrass_section_g(w))
	f_prime_dict = Dict(fp[1] => fp[2] for fp in f_primes)
	g_prime_dict = Dict(gp[1] => gp[2] for gp in g_primes)
	f_prime_dict = factor(weierstrass_section_f(w)).fac
	g_prime_dict = factor(weierstrass_section_g(w)).fac

but that is using internals. I think what we really should do is change the getindex method for Fac to return 0 for missing keys instead of raising an error (as suggested in Nemocas/AbstractAlgebra.jl#1959), then you could just do this:

Suggested change

	f_primes = factor(weierstrass_section_f(w))
	g_primes = factor(weierstrass_section_g(w))
	f_prime_dict = Dict(fp[1] => fp[2] for fp in f_primes)
	g_prime_dict = Dict(gp[1] => gp[2] for gp in g_primes)
	f_primes = factor(weierstrass_section_f(w))
	g_primes = factor(weierstrass_section_g(w))

and replace get(f_prime_dict, d_prime[1], 0) by f_prime[d_prime[1]].

[fingolfin]

You can use nested tuples to make this nicer to read:

Suggested change

	for (i, d_prime) in enumerate(nontrivial_d_primes)
	f_order = get(f_prime_dict, d_prime[1], 0)
	g_order = get(g_prime_dict, d_prime[1], 0)
	d_order = d_prime[2]
	ords = (f_order, g_order, d_order)
	kodaira_types[i] = (ideal([d_prime[1]]), ords, _kodaira_type(ideal([d_prime[1]]), ords, w))
	end
	for (i, (p, n)) in enumerate(nontrivial_d_primes)
	f_order = get(f_prime_dict, p, 0)
	g_order = get(g_prime_dict, p, 0)
	d_order = n
	ords = (f_order, g_order, d_order)
	I = ideal([p])
	kodaira_types[i] = (I, ords, _kodaira_type(I, ords, w))
	end

You could also avoid factoring f and g and do this instead:

Suggested change

	for (i, d_prime) in enumerate(nontrivial_d_primes)
	f_order = get(f_prime_dict, d_prime[1], 0)
	g_order = get(g_prime_dict, d_prime[1], 0)
	d_order = d_prime[2]
	ords = (f_order, g_order, d_order)
	kodaira_types[i] = (ideal([d_prime[1]]), ords, _kodaira_type(ideal([d_prime[1]]), ords, w))
	end
	f = weierstrass_section_f(w)
	g = weierstrass_section_g(w)
	for (i, (p, d_order)) in enumerate(nontrivial_d_primes)
	f_order = valuation(f, p)
	g_order = valuation(g, p)
	ords = (f_order, g_order, d_order)
	I = ideal([p])
	kodaira_types[i] = (I, ords, _kodaira_type(I, ords, w))
	end