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 diagnostics #34

Merged
merged 445 commits into from
Jun 13, 2024
Merged

Conversation

CopyDemon
Copy link
Contributor

Fixes
Fix the UI panel layout, remove the variables panel, and change that panel to a multi-task panel. Now it can display diagnostics.

Proposed changes:
Create a diagnostics panel to display diagnostics.

Notice
Now the diagnostics only run on fixed JSON data, later when the API is up to date, will rewrite some functions in diagnostics.

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@CopyDemon CopyDemon requested a review from dangunter October 24, 2023 21:40
@CopyDemon CopyDemon self-assigned this Oct 24, 2023
Copy link
Member

@dangunter dangunter left a comment

Choose a reason for hiding this comment

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

I assume this will be incorporated in a future PR that uses real/live diagnostic info. Should we move this PR back to draft?

@CopyDemon CopyDemon marked this pull request as draft October 30, 2023 17:30
@CopyDemon
Copy link
Contributor Author

CopyDemon commented Oct 30, 2023

@dangunter you are right, I just turned this PR into a draft, and start to work on serving the new UI with FastAPI and do some mockups for diagnostics. Once the API part is cleaned up and UI is approved, I can come back and update the UI template in code. if need some tests I still can use the old diagnostics UI template in this PR.

For now

@lbianchi-lbl lbianchi-lbl added the Priority:Normal Normal Priority Issue or PR label Jan 18, 2024
@lbianchi-lbl
Copy link
Contributor

The current plan is to merge #43 first and then this one (which is also expected to resolve the current CI failures).

Comment on lines 46 to 52
match request.function_name:
case "display_underconstrained_set":
dt.display_underconstrained_set(stream=output_stream)
case "display_constraints_with_large_residuals":
dt.display_constraints_with_large_residuals(stream=output_stream)
case "display_potential_evaluation_errors":
dt.display_potential_evaluation_errors(stream=output_stream)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
match request.function_name:
case "display_underconstrained_set":
dt.display_underconstrained_set(stream=output_stream)
case "display_constraints_with_large_residuals":
dt.display_constraints_with_large_residuals(stream=output_stream)
case "display_potential_evaluation_errors":
dt.display_potential_evaluation_errors(stream=output_stream)
func = getattr(dt, request.function_name, None)
if func is None:
...
# TODO handle error
func(stream=output_stream)

@CopyDemon CopyDemon mentioned this pull request Mar 11, 2024
@CopyDemon CopyDemon marked this pull request as ready for review April 3, 2024 19:55
@dangunter
Copy link
Member

@CopyDemon Could you rebase this on main? I see some merge conflicts there

@CopyDemon
Copy link
Contributor Author

Could you rebase this on main? I see some merge conflicts there

Working on it now

dangunter and others added 29 commits May 13, 2024 09:41
…ooks like Playwright was just installed or updated. Please run the following command to download new browsers:
@ksbeattie ksbeattie merged commit 18474b8 into IDAES:add-diagnostics-ui Jun 13, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:Normal Normal Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants