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 #6022 srw scale wavefront during sim #6186

Merged
merged 96 commits into from
Sep 18, 2023

Conversation

mkeilman
Copy link
Contributor

@mkeilman mkeilman commented Aug 1, 2023

Putting this in for review though I'm not quite sure about it. It does load the resized plots quicker. Further notes:

  • Because all the logic for resizing the data comes from the intensity files, this copies that functionality into the jinja template
  • For the same reason the wavefront files behave as they did previously, not as per the posited pseudo-code
  • The copying is done with inspect - copied instead of replaced because other 3d plots use the functions
  • The copied functions refer to pkdc and numpy, so the imports are duplicated in a few places
  • There were race conditions between writing and reading the intensity; the reads are now under the same retry block as the other reports

One other item: I had trouble with the example provided in the issue. The 1st watchpoint plot renders, some time after which I get a possible out of memory error. I can't test in production because the plot fails, so I don't know if it's due to an error I've introduced. I'm not sure how to find that out.

@mkeilman mkeilman marked this pull request as ready for review August 31, 2023 02:34
@mkeilman
Copy link
Contributor Author

Latest changes:

  • Changing only the mesh of the wavefront did not work (as suspected). A dive into srw netted ResizeElecFieldMesh() which seems to be the right tool. As the comments to that function explain, it modifies the input wavefront in place
  • Resizing the wavefront ignores various plot parameters such as intensityPlotsWidth so that the wavefront has enough information to reproduce everything. This implies that only the size changes around the max message length affect the size of the wavefront file

@mkeilman mkeilman requested a review from moellep August 31, 2023 02:56
@moellep
Copy link
Member

moellep commented Sep 14, 2023

This is looking really good. I think we can be a bit more aggressive on the scaling - I'll look at this and update this PR.

- clean up preprocess files after execution
@moellep
Copy link
Member

moellep commented Sep 16, 2023

@mkeilman I've made a few changes to the max dimension calculations with 1c2841f
Give it a test with your development environment and then it is ready to go.

@mkeilman
Copy link
Contributor Author

@mkeilman I've made a few changes to the max dimension calculations with 1c2841f Give it a test with your development environment and then it is ready to go.

"sim with large watchpoint files" is giving me "response too large to send" for the 2nd plot with max plot dimension set to original size

@moellep
Copy link
Member

moellep commented Sep 18, 2023

Yes, you have to set the SIREPO_JOB_MAX_MESSAGE_BYTES=2g (which matches production), and also make sure the fix for #6312 is in your source tree.

@mkeilman
Copy link
Contributor Author

Yes, you have to set the SIREPO_JOB_MAX_MESSAGE_BYTES=2g (which matches production), and also make sure the fix for #6312 is in your source tree.

got it, looks good

@moellep moellep enabled auto-merge (squash) September 18, 2023 21:58
@moellep moellep merged commit 8cc5637 into master Sep 18, 2023
2 checks passed
@moellep moellep deleted the 6022-srw-scale-wavefront-during-sim branch September 18, 2023 22:30
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.

3 participants