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 results label in simulation #1929

Merged
merged 9 commits into from
Jan 11, 2025
Merged

Fix results label in simulation #1929

merged 9 commits into from
Jan 11, 2025

Conversation

grzanka
Copy link
Contributor

@grzanka grzanka commented Jan 3, 2025

This pull request includes several updates to the src/services/ShSimulatorService.tsx file to improve the handling of references between estimators and their corresponding outputs and filters. The most important changes include the addition of new functions to rebuild these references and enhancements to error handling and logging.

Enhancements to reference handling:

  • Added detailed JSDoc comments and a new function recreateRefToScoringManagerOutputs to rebuild references between estimators and their corresponding scoring manager outputs.
  • Introduced the recreateRefToFilters function to rebuild references between estimators, their pages, and filters by linking filter UUIDs with actual filter objects.
  • Enhanced the recreateRefsInResults function to include detailed error handling and logging when input JSON or estimators are undefined.

Improvements to ShSimulation component:

  • Modified the ShSimulation component to conditionally recreate references in results based on the availability of editor project data, ensuring proper handling of user-uploaded files. [1] [2]
  • Removed unnecessary console logging for failed or canceled job statuses to clean up the code.
    Related to Results gets wrong label #1927

It seems the source of the bug is here:
26e4159#diff-2179512f339e13f570cb0371ad115ca927d2e7a48929797fd97671033ae6aa05R382-R390

it came with PR #1790 merged on 3.12.2024


For more details, open the Copilot Workspace session.

@grzanka grzanka self-assigned this Jan 3, 2025
grzanka and others added 7 commits January 3, 2025 18:33
…r` and its `outputs` field

* Add console logs for `inputJsonForThisEstimator` and its `outputs` field
…ts` field

* **Recreate references functions**
  - Document `recreateRefsInResults`, `recreateRefToFilters`, and `recreateRefToScoringManagerOutputs` functions

* **Error message**
  - Fix typo in error message from "esitamtors" to "estimators"

* **Input JSON handling**
  - Make a copy of `jobInputs.input.inputJson` and assign it to `inputJsonForThisEstimator`
  - Ensure `inputJsonForThisEstimator` contains a field called `outputs`
  - Pass a single-element list as `outputs` to `recreateRefsInResults`
  - Dump `inputJsonForThisEstimator` to the console, particularly its `outputs` field
…ass filtered outputs

* Make a copy of `jobInputs.input.inputJson` and assign it to `inputJsonForThisEstimator`
* Ensure `inputJsonForThisEstimator` contains a field called `outputs`
* Pass a single-element list as `outputs` to `recreateRefsInResults`
* Update `data` object and `resolve` logic to handle the new `inputJsonForThisEstimator`
…gging

* Make a copy of `jobInputs.input.inputJson` and assign it to `inputJsonForThisEstimator`
* Ensure `inputJsonForThisEstimator` contains a field called `outputs`
* Pass a single-element list as `outputs` to `recreateRefsInResults`
* Add logging for `inputJsonForThisEstimator` in the console
* Add debugging logging to console in `recreateRefToScoringManagerOutputs`, `recreateRefToFilters`, and `recreateRefsInResults`
* Add comments explaining code logic in `recreateRefToScoringManagerOutputs`, `recreateRefToFilters`, and `recreateRefsInResults`
@matuszsmig
Copy link
Contributor

During running simulation from Input Files tab with estimators tab names defined without underscore "_":
image
Underscore appears in Results tab for defined eariler estimators names.
image

@matuszsmig
Copy link
Contributor

Order of defined estimators in input files tab isn't preserved in Results tab as we can see on screenshots above.

@matuszsmig
Copy link
Contributor

For Pages labels seems to work properly

@grzanka
Copy link
Contributor Author

grzanka commented Jan 10, 2025

During running simulation from Input Files tab with estimators tab names defined without underscore "_": image Underscore appears in Results tab for defined eariler estimators names. image

Does it happen on version in master as well ?

@grzanka
Copy link
Contributor Author

grzanka commented Jan 10, 2025

Order of defined estimators in input files tab isn't preserved in Results tab as we can see on screenshots above.

If we run from input files, then there is no way to preserve order of estimators, so that is indended.

@grzanka
Copy link
Contributor Author

grzanka commented Jan 10, 2025

During running simulation from Input Files tab with estimators tab names defined without underscore "_": image Underscore appears in Results tab for defined eariler estimators names. image

I cannot reproduce that - how was it executed ? On yap-dev ? On celery or on Ares ?
Can you put here the JSON of the project, so I could download it and run it myself ?

@matuszsmig
Copy link
Contributor

During running simulation from Input Files tab with estimators tab names defined without underscore "_": image Underscore appears in Results tab for defined eariler estimators names. image

I cannot reproduce that - how was it executed ? On yap-dev ? On celery or on Ares ? Can you put here the JSON of the project, so I could download it and run it myself ?

This behaviour is on both master and this branch. With local backend with celery and frontend.

To reproduce it i load some example, change order and names of Pages and Esimators:
image
Go to Input files, generate from editor, run with this input files, then go to Simulations and when its ready to see results there are estiamtors names with "_":
image

This is JSON:

proton-pencil-beam-in-water-result.json (5).json

@grzanka
Copy link
Contributor Author

grzanka commented Jan 10, 2025

This behaviour is on both master and this branch. With local backend with celery and frontend.

which backend version are you running (are you on recent master?) ? is it docker of local deployment ?

@grzanka grzanka requested a review from matuszsmig January 10, 2025 18:01
matuszsmig
matuszsmig previously approved these changes Jan 11, 2025
const refsInResults =
jobInputs?.input.inputJson &&
recreateRefsInResults(jobInputs.input.inputJson, estimator);
// if editor project data (with filter definions etc) is available, recreate references in results
Copy link
Contributor

Choose a reason for hiding this comment

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

definitions - typo

@grzanka grzanka merged commit a2c8c2c into master Jan 11, 2025
9 checks passed
@grzanka grzanka deleted the grzanka/fix-results-label-2 branch January 11, 2025 11:35
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