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

incorporate new transforms module #495

Open
donald-e-boyce opened this issue Feb 2, 2023 · 5 comments
Open

incorporate new transforms module #495

donald-e-boyce opened this issue Feb 2, 2023 · 5 comments
Labels
HEDM issues pertaining the HEDM workflow

Comments

@donald-e-boyce
Copy link
Collaborator

Incorporate work on transforms done by Oscar Villellas. There are some API changes.

@psavery
Copy link
Collaborator

psavery commented Feb 23, 2023

I believe this is the relevant repository: https://github.com/ovillellas/xrd-transforms

Depending on how extensive the API changes are, we may be able to have some conversion functions that use the old API and automatically convert to the new API. If we do this well enough, we may not even need to change any of the old function calls. It would be great to break as few things as possible!

@psavery psavery added the HEDM issues pertaining the HEDM workflow label Feb 24, 2023
@psavery
Copy link
Collaborator

psavery commented Mar 16, 2023

I just performed some profiling for the monolithic multiruby dexelas example to help us determine which methods from the xrd-transforms repository we should target first for both integration and testing. It appears to me that fit-grains will benefit a lot more from the xrd-transforms than find-orientations. More about that below.

NOTE: I turned off all multiprocessing and multithreading for the profiling, so that we can see specifically which transform functions are taking up the most time. This profiling was thus done with a single process and a single thread (the methods run faster when we enable parallelism).

Find Orientations

I believe that find-orientations won't benefit much from the xrd-transforms. You can see the whole profile here: find_orientations_profile.txt

Out of the ~115 seconds that find-orientations took, the transforms function that took the most time was detectorXYToGvec at ~3 seconds (~2.6% of the time). The calls that took the most time were in the _run_histograms function. In fact, the one thing that is taking up most of the time is the thresholding for generating the eta omega maps right here. This is because a deep copy of every image is made and then modified.

If we modify the settings so that there is no threshold, then the total time taken is ~57 seconds (results here: find_orientations_no_threshold_profile.txt). In this case, a lot of the time is being taken up in numpy, fast_histogram, and scipy methods. The detectorXYToGvec takes up ~5.5% of the time.

However, fit-grains may benefit a lot from the xrd-transforms.

Fit Grains

For fit-grains, the total run time was ~430 seconds, and the amount of time the methods anglesToGVec, gvecToDetectorXYArray, and makeOscillRotMatArray took were 20.8%, 16.3%, and 7.9%, respectively (for a total of ~45% of the time). Profile results are here:
fit_grains_profile.txt

Still, calling numpy.zeros() took up 23% of the time in fit-grains, which makes me wonder if there's a way to improve that (by calling numpy.zeros() less often or with a dtype that uses less memory).

For the cumulative profile (which includes the time taken both in the function and the functions that the function calls), the vast majority of the time was spent in pull_spots() (96% of the time) and _project_on_detector_plane (49% of the time).

Conclusion

I think it's best for us to try to migrate over anglesToGVec, gvecToDetectorXYArray, and makeOscillRotMatArray first and see the new performance afterward.

@donald-e-boyce
Copy link
Collaborator Author

donald-e-boyce commented Mar 16, 2023 via email

@psavery
Copy link
Collaborator

psavery commented Jun 9, 2023

@saransh13 we actually didn't integrate all of the new transforms functions, but only the ones that were relevant for running fit-grains. As such, I'll reopen this.

@psavery psavery reopened this Jun 9, 2023
@ZackAttack614
Copy link
Collaborator

@psavery please let me know if this can be closed now, or if we need to migrate anything else. I would like to knock this out soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HEDM issues pertaining the HEDM workflow
Projects
None yet
Development

No branches or pull requests

4 participants