#19136: NO and NU graphs
-------------------------+-------------------------------------------------
Reporter: | Owner:
dimpase | Status: needs_work
Type: | Milestone: sage-6.9
enhancement | Resolution:
Priority: major | Merged in:
Component: graph | Reviewers: Nathann Cohen
theory | Work issues:
Keywords: | Commit:
Authors: Dima | 79ce567d62ac693b0abe28b659cde2e16abdeb09
Pasechnik | Stopgaps:
Report Upstream: N/A |
Branch: |
u/dimpase/NONU |
Dependencies: |
#19098, #19180 |
-------------------------+-------------------------------------------------
Comment (by dimpase):
Replying to [comment:23 ncohen]:
> - `graphs.SymplecticGraph` does not exist. It is not enough to create a
> deprecation alias, it must also be available in `graphs.<tab>`.
done
>
> - In `NonisotropicOrthogonalPolarGraph`. Replace
> {{{
> else:
> if not perp is None:
> <code>
> else:
> }}}
> with
> {{{
> elif not perp is None:
> <code>
> else:
> }}}
> No need to indent everything twice.
done
>
> - same function: `q` does not appear in the INPUT block
done
>
> - same function: move the checks on q (prime power) and sign (+-) to the
> beginning of the function (not in a 'if' block). This function builds
a
> (dense) graph, we are not at a level where we try to minimize the
amount of
> unnecessary 'if'.
done
>
> - REFERENCE block happens at the end of the docstring
done (moved REFERENCE twice)
>
> - Remove the commented code that your functions contain
hmm, where? I've better worded a comment that looked as code, but
otherwise...
>
> - Why 5 `is_NO*` functions instead of one `is_NO` ? I remember a comment
of
> yours about copy/paste...
it's because the arithmetic properties of parameters are quite different
in each of these
cases. Each has a different algorithm coded.
>
> - If your function is slow it is because of that line
> {{{
> G = Graph([V,lambda x,y:P(vector(x),vector(y))==0],loops=False)
> }}}
> When you do this, 'vector' is called 'binomial(n,2)' times, and
`vector` is
> *SLOW*. In a previous branch of yours, I added a commit to remove this
in
> order to gain a large speedup (do you remember)? Try to find a way to
avoid
> vectors. If you cannot, then instead of building `n^2` vectors you
should turn
> everything into a vector *once*, and work on vectors instead. Observe
that
> vectors have a `.set_immutable` method.
OK, I did the latter. Totally getting rid of `vector()` in these cases
seems tricky.
Anyhow, it was quite an improvement.
>
> Then, perhaps we will be able to remove those `# long time` everywhere
in
> `_orthogonal_polar_graph` (and elsewhere).
done
>
> - Same in `UnitaryPolarGraph`.
done (as above)
>
> Next time, please think of the reviewer when you built your commits.
When you
> save minutes by not doing it, the reviewer loses hours trying to figure
out what
> exactly you did.
sorry for the mess...
--
Ticket URL: <http://trac.sagemath.org/ticket/19136#comment:28>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
