Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add experimental dashboard to monitor simulations #6137

Merged
merged 8 commits into from
Jul 15, 2024

Conversation

nilsvu
Copy link
Member

@nilsvu nilsvu commented Jun 29, 2024

Proposed changes

This takes the status data and displays it in a web browser for all running jobs on the cluster, along with lots of plots to show the current state of BBH simulations. It's experimental, in that to use it you have to pip install streamlit by yourself and run bin/python-spectre -m streamlit run tools/Status/Dashboard.py to start the dashboard. There are some instructions in the file.

Here's a glance at what it looks like:

Bildschirmfoto 2024-06-29 um 14 04 37

I found this so useful when monitoring BBH simulations that I decided to open this experimental PR.

Upgrade instructions

Code review checklist

  • The code is documented and the documentation renders correctly. Run
    make doc to generate the documentation locally into BUILD_DIR/docs/html.
    Then open index.html.
  • The code follows the stylistic and code quality guidelines listed in the
    code review guide.
  • The PR lists upgrade instructions and is labeled bugfix or
    new feature if appropriate.

Further comments

Copy link
Contributor

@knelli2 knelli2 left a comment

Choose a reason for hiding this comment

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

These are all optional really. Just some things I noticed. We can't test any of this so once you feel it's ready we can just merge

Comment on lines 135 to 136
with open(outfile, "r") as open_outfile:
st.code("".join(["...\n"] + open_outfile.readlines()[-30:]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Any way to make this scrollable?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried loading the full file but that broke the server :D Added a comment

Comment on lines 176 to 177
# Refresh the page regularly
if st.toggle("Auto-refresh", True):
Copy link
Contributor

Choose a reason for hiding this comment

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

I found that if there is an error rending one of the plots of an executable status, then the auto-refresh is somehow disabled. Clicking on a different run brings it back. Any ideas on what to do for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ok I moved the auto refresh further up in the script, that should fix it

Comment on lines 73 to 74
job: A dictionary of job information, including the input file and
working directory. See the 'spectre.tools.Status.status' function
Copy link
Contributor

Choose a reason for hiding this comment

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

fetch_status or render_status to be explicit?

Comment on lines 134 to 136
"L2Norm(PointwiseL2Norm(GaugeConstraint))",
"L2Norm(PointwiseL2Norm(ThreeIndexConstraint))",
"L2Norm(PointwiseL2Norm(FourIndexConstraint))",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use

[col for col in df.columns if "Constraint" in col]

so that it will work if people stray from the pipeline a bit?

Comment on lines 214 to 216
# Eccentricity
# (can be enabled once EccentricityControl is available)
# st.subheader("Eccentricity")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you've updated this section since you opened this. Wanna add those changes in?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's enabled now since #5895 was merged

Comment on lines 250 to 273

class EllipticStatus(ExecutableStatus):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you've also added a dashboard for the elliptic one too since you opened this. Wanna add that? I think the only thing is figuring out how to show the latest control step and not just the first one.

Copy link
Member Author

@nilsvu nilsvu Jul 3, 2024

Choose a reason for hiding this comment

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

I added the elliptic dashboard. It only shows the convergence for the first control step atm, and also plots the convergence of the control parameter. I'll probably combine these somehow, but haven't decided how yet. Also AMR iterations are still missing from the status.

@nilsvu nilsvu force-pushed the dashboard3 branch 3 times, most recently from 249873b to 53f1077 Compare July 3, 2024 22:16
Copy link
Contributor

@knelli2 knelli2 left a comment

Choose a reason for hiding this comment

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

Everything else looks ok

Comment on lines 122 to +123
# Compute separation
num_obs = len(ObjectA_centers[:, 0])

if len(ObjectB_centers[:, 0]) < num_obs:
num_obs = len(ObjectB_centers[:, 0])
num_obs = min(len(ObjectA_centers[:, 0]), len(ObjectB_centers[:, 0]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this commit and the one before should be in a separate PR since they are a bug fix? That way they don't get buried in an experimental PR

@knelli2
Copy link
Contributor

knelli2 commented Jul 15, 2024

@nilsvu once the tests are fixed, we can merge this

nilsvu and others added 2 commits July 15, 2024 14:15
Make sure isort always treats the `spectre` imports as
first party. Before this fix, isort could get confused if the
source directory was not named "spectre".
Allows to call the function separately from the CLI.
@nilsvu nilsvu force-pushed the dashboard3 branch 2 times, most recently from 3876903 to 09402e2 Compare July 15, 2024 21:24
@nilsvu
Copy link
Member Author

nilsvu commented Jul 15, 2024

Ok I squashed and rebased. Thanks for reviewing!

@knelli2 knelli2 merged commit 7ae00e7 into sxs-collaboration:develop Jul 15, 2024
22 checks passed
@knelli2 knelli2 added the cli/pybindings Command line interface & Python bindings label Aug 3, 2024
@nilsvu nilsvu deleted the dashboard3 branch August 3, 2024 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli/pybindings Command line interface & Python bindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants