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.

Sign up for GitHub

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

Jump to bottom

Reduce memory consuption in axis_world_coords (again) #798

Merged
merged 4 commits into from
Jan 13, 2025
Merged

Reduce memory consuption in axis_world_coords (again) #798

merged 4 commits into from
Jan 13, 2025

Conversation

Cadair
Copy link
Member

@Cadair Cadair commented Dec 17, 2024

This PR works, as an alternative to #780 by not generating mesh coords for unneeded pixel axes.

i.e. if the pixel axes are not correlated with a requested world axis then don't generate the coords in the mesh.

@Cadair Cadair marked this pull request as draft December 17, 2024 17:39
@Cadair
Copy link
Member Author

Cadair commented Dec 17, 2024

This PR needs tests which test at least with some DKIST or IRIS WCSes that you can get low-memory axis world coords for uncorrelated axes, and as a counterpoint that correlated axes still work properly.

For example, I have tested that axis_world_coords("time") works fine for this dataset but not that axis_world_coords("lat") works for instance (which should still generate a large grid).

Cadair
Cadair commented Dec 17, 2024
View reviewed changes
ndcube/ndcube.py Outdated Show resolved Hide resolved
@nabobalis nabobalis added this to the 2.3.0 milestone Dec 17, 2024
@nabobalis
Copy link
Contributor

I do question why I bother to do any programming if its always wrong.

This solution works:

Command line: /home/nabil/.mamba/envs/iris-dev/bin/memray run -f -o output.bin examples/umbral_flashes.py
Start time: Tue Dec 17 2024 11:08:16 GMT-0800 (Pacific Standard Time)
End time: Tue Dec 17 2024 11:08:42 GMT-0800 (Pacific Standard Time)
Duration: 0:00:26.707000
Total number of allocations: 6674037
Total number of frames seen: 17921
Peak memory usage: 2.3 GiB
Python allocator: pymalloc

The biggest allocations are from loading the data:

Screenshot_20241217_111209

@DanRyanIrish
Copy link
Member

This is great news!

@Cadair
Copy link
Member Author

Cadair commented Dec 19, 2024

@nabobalis I can't add a visp WCS without a dkist dependency, I assume you can't either?

@nabobalis
Copy link
Contributor

@nabobalis I can't add a visp WCS without a dkist dependency, I assume you can't either?

I am the same

@nabobalis
Copy link
Contributor

Could we try Wills WCS from his issue?

@nabobalis nabobalis force-pushed the maybe_this_will_work branch from deee62c to 72d8859 Compare January 8, 2025 00:43
nabobalis
nabobalis reviewed Jan 8, 2025
View reviewed changes
ndcube/conftest.py
@@ -519,6 +560,25 @@ def ndcube_3d_rotated(wcs_3d_ln_lt_t_rotated, simple_extra_coords_3d):
return cube



@pytest.fixture
def ndcube_3d_coupled(wcs_3d_ln_lt_l_coupled):
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this is good.

@nabobalis nabobalis marked this pull request as ready for review January 8, 2025 00:44
@nabobalis nabobalis force-pushed the maybe_this_will_work branch 2 times, most recently from ae8bac4 to e15bc13 Compare January 8, 2025 01:41
@Cadair Cadair force-pushed the maybe_this_will_work branch 2 times, most recently from 17ec1ec to ff9db90 Compare January 8, 2025 15:40
@Cadair Cadair mentioned this pull request Jan 8, 2025
Merged
@Cadair Cadair force-pushed the maybe_this_will_work branch from ff9db90 to c440645 Compare January 8, 2025 15:42
@Cadair
Copy link
Member Author

Cadair commented Jan 8, 2025

I would be nice to get a test which tests the 2D usecase.

@nabobalis
Copy link
Contributor

How does one do that?

@Cadair Cadair force-pushed the maybe_this_will_work branch from c440645 to 324413f Compare January 13, 2025 14:52
Cadair and others added 3 commits January 13, 2025 15:01
@Cadair
use memray for a test
9a67d59
@Cadair
Do not generate mesh coords for unneeded pixel axes
323960a 
If the pixel axes are not correlated with a requested world axis then
don't generate the coords in the mesh
@nabobalis @Cadair
changelog update
639d7f9
@Cadair Cadair force-pushed the maybe_this_will_work branch 2 times, most recently from eb2cca2 to 71c7e86 Compare January 13, 2025 15:08
@Cadair Cadair force-pushed the maybe_this_will_work branch from 71c7e86 to 1a5bd72 Compare January 13, 2025 15:24
@nabobalis nabobalis merged commit 1c9faa1 into sunpy:main Jan 13, 2025
19 of 20 checks passed
@Cadair Cadair deleted the maybe_this_will_work branch January 13, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nabobalis nabobalis nabobalis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Cadair @nabobalis @DanRyanIrish