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

Reduce memory usage when applying supertranslations by a factor of 6 #82

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dongzesun
Copy link

Applying the supertranslations to the abd quantities one by one to reduce memory usage.

@moble
Copy link
Owner

moble commented Nov 30, 2022

First, I'll reiterate that this is not what I think you should be doing. Like I said in my previous email, if you only need to transform the strain, then transforming all the extra parts of the ABD will make your code 6 times slower for no reason — and still use more memory than you need. For your use case, you just need the ABD to find the translation to the super-rest frame; then, you can just transform the strain itself without using an ABD at all for the transformation. Basically, split the parts above and below this line into two separate functions, and you just need the first function. (Also, you need to make sure you're not using a crazy ell_max.)

Now, more generally, for other people who want to transform an ABD, I'm not sure that this is the right way to go either. In particular, when n_times and ell_max are reasonable, memory consumption isn't usually the problem; the problem is usually speed. And in that case, it's important to prioritize the splines, which is what the existing code does. If you want to move forward with this, I'd need to see some evidence that this doesn't slow the whole process down to a crawl for more common use cases.

If this is the better way to go even when n_times and ell_max are reasonable, you should also remove a lot of that code duplication. You've introduced five big blocks of nearly identical code. That should be wrapped up into a function instead. Also, the use of fprime_of_timenaught_directionprime is redundant.

@dongzesun
Copy link
Author

Thanks for the review. But I also need to find the translations that map to the PN BMS frame, so at least psi_2 is needed apart from the strain. Should I write another function that only transforms psi_2 and strain?

@duetosymmetry
Copy link
Contributor

Tangentially related to #81

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