Skip to content

Replace static PNG isogeny graphs with interactive Cytoscape.js#6874

Open
edgarcosta wants to merge 12 commits intoLMFDB:mainfrom
edgarcosta:isogenygraphs
Open

Replace static PNG isogeny graphs with interactive Cytoscape.js#6874
edgarcosta wants to merge 12 commits intoLMFDB:mainfrom
edgarcosta:isogenygraphs

Conversation

@edgarcosta
Copy link
Member

@edgarcosta edgarcosta commented Feb 13, 2026

Addresses LMFDB#6686 (excessive whitespace) and LMFDB#2801 (interactive graphs).
Static matplotlib-rendered PNGs are replaced with Cytoscape.js graphs
featuring clickable nodes (navigate to curve pages), hover tooltips
with curve metadata, and optimal curve highlighting. The properties
sidebar now uses a compact server-generated SVG. Trivial (single-curve)
isogeny classes now display their graph as well.
Compute container dimensions from actual node positions instead of
using a fixed 600x400 box. Eliminates excessive vertical whitespace
for small/horizontal graphs (e.g. 2-node classes).
For ECNF isogeny classes with topologies not handled by make_graph
(e.g. 18 curves), fall back to cytoscape's cose layout instead of
placing all nodes at origin. SVG sidebar uses circular layout as
fallback. Add 'Number of curves' to ECNF properties sidebar.
- Extract CDN scripts into shared cytoscape_scripts.html template
- Add layout dropdown with 18 options (7 built-in + 11 extensions)
  including cola, dagre, klay, elk variants, fcose, etc.
- Default to preset layout when make_graph provides positions,
  elk(stress) otherwise; hide preset option when not available
- Sidebar SVG: use layout_spring() fallback, remove labels, use
  smaller filled dots for a cleaner compact appearance
@rvisser7
Copy link
Contributor

Many thanks @edgarcosta, this looks awesome!! I've just added a few small comments below, after doing some testing:

  • There still seems to be some excess whitespace in the properties box for small isogeny graphs. E.g. see the properties box for the isogeny classes 26.b, 11.a, and 27.a. We should probably also reduce the whitespace for the trivial isogeny class, e.g. 37.a, (though at least here the displayed whitespace is symmetric both above and below the graph).

  • While it's a cool feature, do we necessarily want to enable draggable nodes for simple graphs where we already have a fixed hardcoded layout - in particular for elliptic curves isogeny graphs over Q? We do have this for subgroup diagrams, though I think this is primarily for those cases where the diagrams can get large/complex with many subgroups. For elliptic curves over Q, all the possible isogeny graphs have been classified (in particular all such graphs are planar and have size at most 8, e.g. see this paper). So I don't really think having interactive draggable nodes (and having all the layout dropdown options) are particularly necessary here, especially if make_graph() already has a preset structure for the graph . (but of course I agree with having the nodes linked in all cases!)

  • If we do enable draggable nodes, then at moment it seems it's possible for the user to drag the nodes outside the visible window, which shouldn't be possible.

  • As mentioned in issue #6686, I do think we should try to uniformize the distance between adjacent nodes, relative to the node size. E.g. While I think the default spacing shown in 14.a looks very nice, the spacing for the square isogeny graphs (e.g. 20.a ) I think should be a bit smaller, and match that of the first square in 14.a. I assume this can easily be fixed by just adjusting the hardcoded coordinates given in lmfdb/elliptic_curves/isog_class.py#L253

  • Similarly, I imagine the issue with https://olive.lmfdb.xyz/EllipticCurve/Q/30/a/ can also be solved by just adjusting the hardcoded coordinates given in lmfdb/elliptic_curves/isog_class.py#L283

  • For elliptic curves over Q, we're currently labelling the nodes just as a1, a2, a3, etc., but for elliptic curves over number fields, we're giving each node the full label. Probably we should uniformize? I think the full label is ok for small conductor curves, though once the conductor (or conductor norm) gets very big, the label can overflow the node boundaries, e.g see: https://olive.lmfdb.xyz/EllipticCurve/2.0.3.1/145236.8/h/

  • Perhaps we can also add information about the torsion structure, j-invariant, etc., when hovering over the nodes for the number field isogeny graphs?

  • (Optional) Maybe we can reintroduce colouring-in the nodes of the isogeny graph, as done for the current isogeny graphs? I like the pinkish peach colour that beta currently has. :)

But overall, this is a definitely great improvement to what we currently have, thanks again!! :)

@roed314
Copy link
Member

roed314 commented Feb 14, 2026

These are cool @edgarcosta! Following up on a few of Robin's comments:

  • I would use just the short label (like a1) and omit the conductor part.
  • I agree that we probably don't need draggable notes in this application, but there are other cases where it would be helpful.
  • the popups with info are great!

As we refine this, it would be nice to see how the subgroup diagrams look in this system.

@edgarcosta
Copy link
Member Author

edgarcosta commented Feb 14, 2026

Thank you for your comments. Hopefully I will try to address them next Friday.
Here is what I heard, and I thinks is reasonable to address:

  • Reduce vertical white space in sidebar
  • tweak hardcoded positions
  • drop conductor over nfs for labels
  • add information about the torsion structure, j-invariant, etc., when hovering over the nodes for the number field isogeny graphs

Not planning to address, but feel free to submit a patch if it is important for you:

  • disable dragable nodes
  • dragging nodes out of canvas (we could perhaps enable zoom, to recover them)
  • color

also here are other demos of cytoscape: https://js.cytoscape.org/

@JohnCremona
Copy link
Member

This looks very good indeed -- thanks, @edgarcosta !

I don't see the need for having individual vertices draggable. For the larger diagrams (such as the last example shown) I would like the ability to rotate the whole diagram in 3D-graphics style -- this is what you get when you create an isogeny class in Sage directly (implemented by me a while ago) but I never worked out how to store a 3D graphics object which we could display in the LMFDB. This is absolutely not essential.

I agree that we should be consistent in how much of the labels we use to decorate the graph nodes. In fact only the number at the end of the label is varying across the class so a minimal version would be to just have that (perhaps omitting the single '1' if the class is trivial).

Having the pop-up info for the curves in the class is great.

@edgarcosta edgarcosta force-pushed the isogenygraphs branch 2 times, most recently from 126dd4d to ecf753e Compare February 20, 2026 01:34
- Adaptive SVG sizing in sidebar to reduce whitespace for small graphs
- Shrink square graph coordinates to match double-square spacing
- Spread out tent graph (maxdegree=12) to prevent node overlap
- Set position for single-vertex graphs; compact container with no
  layout dropdown
- Use short labels (e.g. "a1") for ECNF graph nodes
- Add torsion and CM to ECNF node tooltips
@edgarcosta edgarcosta marked this pull request as ready for review February 20, 2026 01:37
@havarddj
Copy link
Contributor

havarddj commented Feb 20, 2026

Looks great! Here are a couple of comments:

  1. The max-width for the tooltips should be a bit bigger; also, currently the tooltips are not allowed to expand beyond the parent div, so the right-most tooltip box is often too narrow. I'm sure your LLM can fix that!
Screenshot 2026-02-20 at 16 09 36 2. When there's no torsion, perhaps it should say "[0]" or "None" instead of "[]", e.g. in https://olive.lmfdb.xyz/EllipticCurve/Q/37/a/ 3. A really cool feature would be if hovering over a node would also highlight the corresponding entry in the table below. It shouldn't be too hard, but perhaps it's not worth the extra complexity.

I'll add some comments in the code as well.

tooltip.style.cssText = 'display:none; position:absolute; background:#fff; ' +
'border:1px solid #ccc; border-radius:4px; padding:8px 12px; ' +
'font-size:13px; line-height:1.5; box-shadow:0 2px 8px rgba(0,0,0,0.15); ' +
'z-index:1000; pointer-events:none; max-width:280px;';
Copy link
Contributor

@havarddj havarddj Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's necessary to set max-width (or at least, you can set it quite big without too much harm; I would always prefer the data to be on a single line)

'label': 'data(label)',
'font-size': '11px',
'text-background-color': '#fff',
'text-background-opacity': 0.85,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer 'text-background-opacity: 1', so you don't get the ghost lines

'height': 30,
'shape': 'round-rectangle',
'color': '#333',
'cursor': 'pointer'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line doesn't seem to do anything. Instead, you should change pointer on mouseover events, I'll comment below.

// Position tooltip near the node
var pos = node.renderedPosition();
tooltip.style.left = (pos.x + 15) + 'px';
tooltip.style.top = (pos.y - 10) + 'px';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add $('html,body').css('cursor', 'pointer'); here

return "data:image/png;base64," + quote(b64encode(buf))


def graph_to_cytoscape_json(G):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it makes sense to put this in a separate file?

Comment on lines 268 to 269
var hw = node.width() / 2;
var hh = node.height() / 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add +10 to these to make account for the shadow around the box

@edgarcosta
Copy link
Member Author

@havarddj @rvisser7 please suggest some "modes" to keep

- Move tooltip to document.body to prevent clipping by container overflow
- Use white-space:nowrap instead of max-width for wider tooltips
- Set text-background-opacity to 1 on edge labels to hide ghost lines
- Remove ineffective cursor:pointer CSS; use jQuery on mouseover/mouseout
- Add +10 margin to drag constraint for box shadow
- Format torsion as "Z/2Z x Z/4Z" or "Trivial" instead of raw list
@havarddj
Copy link
Contributor

There seems to be a bug currently where selecting preset -> selecting different layout -> selecting preset again doesn't restore the preset layout.
Here's my ordered list of preferences, checked with https://olive.lmfdb.xyz/EllipticCurve/Q/30/a/

  1. Circle
  2. Elk-stress
  3. Klay
  4. Dagre
  5. Cola

@edgarcosta
Copy link
Member Author

@havarddj where they play a bigger role is in NF e.g. https://olive.lmfdb.xyz/EllipticCurve/4.4.2304.1/1.1/a/

Python sets graph_layouts list, passed to JS via templates.
cytoscape_scripts.html conditionally loads only the CDN scripts
needed for the chosen layouts.  Default set: Preset, Elk-stress,
Circle, Concentric, Klay, Dagre, Cola.
…ring

This ensures the Preset layout option is always available, and the
interactive graph starts with the same coordinates as the sidebar SVG.
…itions

graph_to_cytoscape_json now returns (elements, has_preset).  Python
sets graph_default_layout to 'Preset' when positions are hardcoded
or 'Elk-stress' when falling back to layout_spring.  The default is
passed through to JS so the initial layout and dropdown selection
match.
@edgarcosta edgarcosta requested a review from havarddj February 20, 2026 19:47
Move make_graph, graph_to_cytoscape_json, graph_to_svg, setup_isogeny_graph,
and GRAPH_LAYOUTS into a dedicated module, eliminating ~160 lines of
near-identical code that was duplicated between elliptic_curves/isog_class.py
and ecnf/isog_class.py. The unified make_graph uses ECNF's more defensive
guards (checking both maxdegree and n) while keeping EC/Q's vertex_labels
parameter.
Copy link
Contributor

@havarddj havarddj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

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

Successfully merging this pull request may close these issues.

5 participants