Skip to content

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

Merged
nfaguirrec merged 22 commits into
trunkfrom
plot_energy_landscape
Aug 19, 2026
Merged

Add matplotlib energy landscape plot helper with selectable layouts Unit test included.#415
nfaguirrec merged 22 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!

Comment thread unit_tests/test_tools_plot.py


@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(

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.

I meant add to the top level init.py (see link above) so it can be imported directly from plams

Comment thread src/scm/plams/tools/plot.py Outdated
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.

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.

I would use the Literal type hint as these are the only options, so users with IDEs will get highlighting for an incorrect option

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.

Good Idea! Done! :)

Comment thread src/scm/plams/tools/plot.py Outdated
@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!

@nfaguirrec
nfaguirrec merged commit 37a8f4e into trunk Aug 19, 2026
21 checks passed
@nfaguirrec
nfaguirrec deleted the plot_energy_landscape branch August 19, 2026 15:45
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