a cache for OA/TD/MOLS existence

[ncohen]

Here it is ! Short and efficient, but OA-specific.

Right now it is not linked wth any constructor, it will be done while #16347 is implemented.

The same thing will have to be done later for incomplete_orthogonal_array.

Nathann

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​f23d39c

trac #16460: a cache for OA/TD/MOLS existence

[vdelecroix]

Hi Nathann,

I implemented the changes associated to the following comments in u/vdelecroix/16460.

• the default for min_false should be n+2? No?
• with the current implementation, the _set_OA_cache set a new value to _OA_cache[n] even if nothing has changed. This is bad from the global warming point of view.
• it would be better that the keys and values are only Python int or only Sage Integer but not a mix.

On the other hand, wouldn't it be better to have this in designs_pyx.pyx?

Vincent

[ncohen]

Yo !

• the default for min_false should be n+2? No?

Aahahah right. I had thought about this afterwards and forgot to change it, but I had comforted myself before by figuring out that it had actually no effect on the answers. Because "True" is tested before "False" :-)

• with the current implementation, the _set_OA_cache set a new value to _OA_cache[n] even if nothing has changed. This is bad from the global warming point of view.

And you think that your added "if" have a zero cost ? To me it is rather useless and adds lines for no point...

• it would be better that the keys and values are only Python int or only Sage Integer but not a mix.

I'm not sensible to these problems :-P

On the other hand, wouldn't it be better to have this in designs_pyx.pyx?

Ahahah. We are not there yet. There is a long road in front of us before that this caching mechanism represent a non-negligible part of the computations :-P

Okay... Well, if you insist on having this "best" test I guess it can go although really I don't see the point... Otherwise if you are okay with not including it I would be glad to add to my branch the part of your commit that converts k,n to Python 'int' if it makes you sleep easier.

Nathann

[vdelecroix]

Replying to ncohen:

Yo !

• the default for min_false should be n+2? No?

Aahahah right. I had thought about this afterwards and forgot to change it, but I had comforted myself before by figuring out that it had actually no effect on the answers. Because "True" is tested before "False" :-)

• with the current implementation, the _set_OA_cache set a new value to _OA_cache[n] even if nothing has changed. This is bad from the global warming point of view.

And you think that your added "if" have a zero cost ? To me it is rather useless and adds lines for no point...

• it would be better that the keys and values are only Python int or only Sage Integer but not a mix.

I'm not sensible to these problems :-P

On the other hand, wouldn't it be better to have this in designs_pyx.pyx?

Ahahah. We are not there yet. There is a long road in front of us before that this caching mechanism represent a non-negligible part of the computations :-P

Okay... Well, if you insist on having this "best" test I guess it can go although really I don't see the point... Otherwise if you are okay with not including it I would be glad to add to my branch the part of your commit that converts k,n to Python 'int' if it makes you sleep easier.

Please convert to int and set the default False to n+2. Otherwise, you assume that there is no projective plane except for prime powers.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​60b717f

trac #16460: small fixes

[ncohen]

Done !

Nathann

[vdelecroix]

I set to positive review. But anyway, it will be interesting only when it will be integrated in the code.

Vincent

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:

​f1c7fa1	trac #16295: Typo
​451d359	trac #16295: Merged with 6.3.beta2
​31e84b8	trac #16295: todo note
​6fb1f14	trac #16295: Bugfix
​1653790	trac #16295: Merged with 6.3.beta3
​b66a187	fixed some small language typos and changed MOLS error messages into context that MOLS researchers will expect.
​02f1247	trac #16295: one more doctest
​f969003	trac #16356: MOLS for n=18,57,154,276,298,342
​917dd5c	trac #16356: Merged with #16295
​d7cb52a	trac #16460: Merged with updated #16356