Skip to content

Add matplotlib energy landscape plot helper with selectable layouts Unit test included. - #415

Open
nfaguirrec wants to merge 19 commits into
trunkfrom
plot_energy_landscape
Open

Add matplotlib energy landscape plot helper with selectable layouts Unit test included.#415
nfaguirrec wants to merge 19 commits into
trunkfrom
plot_energy_landscape

Conversation

@nfaguirrec

@nfaguirrec nfaguirrec commented Aug 6, 2026

Copy link
Copy Markdown
Member

Now we can do these things :)

plot_energy_landscape

Also, including the molecules:

plot_energy_landscape_with_molecules

For surfaces, it also works!

plot_energy_landscape_surfaces

@nfaguirrec
nfaguirrec requested a review from dormrod August 6, 2026 14:53
@nfaguirrec nfaguirrec changed the title Add matplotlib energy landscape plot helper with selectable layouts Unit test included. WIP: Add matplotlib energy landscape plot helper with selectable layouts Unit test included. Aug 6, 2026
@nfaguirrec nfaguirrec changed the title WIP: Add matplotlib energy landscape plot helper with selectable layouts Unit test included. Add matplotlib energy landscape plot helper with selectable layouts Unit test included. Aug 6, 2026
@nfaguirrec
nfaguirrec marked this pull request as draft August 6, 2026 20:46
@nfaguirrec nfaguirrec changed the title Add matplotlib energy landscape plot helper with selectable layouts Unit test included. WIP: Add matplotlib energy landscape plot helper with selectable layouts Unit test included. Aug 7, 2026

@dormrod dormrod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks very nice!

A tiny nit-pick is that it looks to me like the numbers cover some of the molecules with a white square, but this is very minor indeed ;)

Also remember to update the CHANGELOG!

remove_text=True,
extensions=["png"],
style="mpl20",
tol=20,

@dormrod dormrod Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These can be a bit flakey, but tol of 20 is quite high so should be ok...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I just copied it from the other tests. So far, it works. But it is a great point to keep in mind, in case at some point the tests fail



@requires_optional_package("matplotlib")
def plot_energy_landscape(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, every plot_* function in src/scm/plams/tools/plot.py has the same decorator, 10 in total. I thought it was on purpose.

$ grep -H -A1 'requires_optional_package("matplotlib")' src/scm/plams/tools/plot.py 
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_band_structure(
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_phonons_band_structure(
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_phonons_dos(
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_phonons_thermodynamic_properties(
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-@requires_optional_package("ase")
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_image_grid(
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_correlation(
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_msd(
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_work_function(
--
src/scm/plams/tools/plot.py:@requires_optional_package("matplotlib")
src/scm/plams/tools/plot.py-def plot_energy_landscape(

connector_color: str = "black",
connector_linestyle: Any = (0, (4, 4)),
label_states: bool = True,
layout: str = "auto",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we help out the user here: Literal["auto", "dfs", "bfs", "longest_path", "force", "crossings"]?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added this in the function's documentation:

  • Use "auto" to compare the available strategies and pick the one with the cleanest connector pattern.
  • Use "dfs" to follow one branch deeply before backtracking, which can resemble a reaction-path view.
  • Use "bfs" to expand level by level from one endpoint, keeping nearby states grouped together.
  • Use "longest_path" to place the main backbone of the network first and then attach side branches around it.
  • Use "force" to apply a simple force-based relaxation that spreads states while reducing visual crowding.
  • Use "crossings" to minimize connector crossings directly, which can help for dense networks.

Comment thread src/scm/plams/tools/plot.py Outdated
state_label = getattr(state, "display_id", state.id)
state_plot_kwargs = dict(molecule_plot_kwargs)
state_plot_kwargs.update(molecule_plot_kwargs_by_state.get(state_label, {}))
plot_molecule(state.molecule, ax=inset_ax, keep_axis=False, **state_plot_kwargs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You might be able to use view method here instead, if you wanted AMSview style molecules instead of ASE style molecules

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have added the parameter molecule_plot_backend: Literal["view", "plot_molecule"] = "view" to the plot_energy_landscape function. Now, the user can choose which backend they prefer

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

By the way, I just realized that molecule_plot_backend="view"cannot be tested in CI because ams is not installed there

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I also added an example examples/EnergyLandscape/EnergyLandscape.ipynb where I show how to use these new functions, including the use of view and plot_molecule

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I also added this option (highlighting states):

image

@nfaguirrec
nfaguirrec marked this pull request as ready for review August 11, 2026 16:27
@nfaguirrec nfaguirrec changed the title WIP: Add matplotlib energy landscape plot helper with selectable layouts Unit test included. Add matplotlib energy landscape plot helper with selectable layouts Unit test included. Aug 11, 2026
@nfaguirrec

nfaguirrec commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

Looks very nice!

A tiny nit-pick is that it looks to me like the numbers cover some of the molecules with a white square, but this is very minor indeed ;)

Also remember to update the CHANGELOG!

Thanks!
In those cases, you can use molecule_y_offset to hide the white squares :)
I already updated the CHANGELOG!

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.

2 participants