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

Fix various experiment header issues #52

Merged

Conversation

jescalada
Copy link
Collaborator

@jescalada jescalada commented Mar 23, 2024

Fixes for G-Research/fasttrackml#1021 and G-Research/fasttrackml#1022.

#1021

This fixed an issue with the console erroring out on empty metrics query:
image

Now we get the desired "No Results" page, and no query is done on backend.

#1022

The issue was that when I wrote the ExperimentSelectionHeader, I did not add the selectedExperimentNames as a state within the MetricsBar. The state was working correctly under normal circumstances because the view was getting refreshed anyways when querying. Empty queries are not sent to the server to begin with, therefore no refresh was happening.

Explanation: Added extra state to MetricsBar and set it on the handler function:

  • onSelectExperimentNamesChange() changes the model and toggles the experiment internally
  • setSelectedExperiments() updates the view to reflect the recent changes

Note: #1021 was also affected by these changes and has been tested as well.

Extra

I fixed a small misalignment in the SelectFormPopper metrics/params.

@jescalada jescalada self-assigned this Mar 23, 2024
@jescalada jescalada changed the title Fix rerender issue with no selected params Fix various experiment header issues Mar 24, 2024
@jescalada jescalada linked an issue Mar 24, 2024 that may be closed by this pull request
@jescalada
Copy link
Collaborator Author

@fabiovincenzi It seems I accidentally removed one of your recent changes to the createAppModel.ts getMetricsData() function during my merge conflict resolution:

These were your original changes:
image

Any ideas on how to reconciliate the changes? 🤔

Copy link
Collaborator

@fabiovincenzi fabiovincenzi 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!

@jescalada jescalada merged commit 582ed30 into G-Research:release/v3.17.5 Mar 29, 2024
3 checks passed
vinayan3 pushed a commit to vinayan3/fasttrackml-ui-aim that referenced this pull request Aug 28, 2024
* Fix rerender issue with no selected params

* Fix metrics error bug

* Fix select form popper styling

* Update createAppModel.ts
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.

Add re-render for experiment status when no params selected Add error message when no params selected
2 participants