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 footprint #16

Closed
wants to merge 2 commits into from
Closed

Reduce memory footprint #16

wants to merge 2 commits into from

Conversation

hogru
Copy link

@hogru hogru commented Mar 27, 2024

Context

I used the new version (1.2) of fcd to calculate the FCD of > 1M molecules.

Issue

In this scenario, my corresponding python script crashed due to a large memory footprint (> 50 GB) on both macOS and Linux. This might be a local issue and depend on the specific pytorch version, among other things.

Suggested resolution

I amended the code in two ways:

(1) Used a different context manager for inference; this does not solve the issue, but was done in addition to ...
(2) Casting the inference result to numpy float32 reduced the memory footprint. I am not sure why this works since the data type without the corresponding line is already float32 and should be without any additional data, such as gradients.

I calculated the FCD for smaller molecule sets of size 100,000 to check whether the FCD value remains the same, which it did in my experiments.

In summary, I consider this to be a minor change which helps to alleviate memory problems, at least in certain configurations.

renzph added a commit that referenced this pull request Mar 29, 2024
@renzph
Multi changes are part of this commit.
1249378 
1. Changes to `get_one_hot`
Problems are given in:
- #14
- #17
- #13

I discarded the changes in the PRs and and added more comprehensive handling of the input data in the
`SmilesDataset` class and the `get_one_hot` function.

2. Imaginary components
Frechet distance calculation fails to work for some cases because of badly conditioned matrices,
as described here #15.

Could not reproduce the error locally, but could do so on colab.

Fixed it in `calculate_frechet_distance` by checking if the first `covmean` computation  is real add a small value to the diagonal.
This made it work for me and I got the same result as the original implementation run locally.

3. Added some more tests and changed to pytest

4. As described in #16
I changed the data type of the activations to float32 in the `get_predictions` function,
which saves memory for larger datasets.
renzph added a commit that referenced this pull request Apr 1, 2024
@renzph
Multiple changes are part of this commit.
3197497 
1. Changes to `get_one_hot`
Problems are given in:
- #14
- #17
- #13

I discarded the changes in the PRs and and added more comprehensive handling of the input data in the
`SmilesDataset` class and the `get_one_hot` function.

2. Imaginary components
Frechet distance calculation fails to work for some cases because of badly conditioned matrices,
as described here #15.

Could not reproduce the error locally, but could do so on colab.

Fixed it in `calculate_frechet_distance` by checking if the first `covmean` computation  is real add a small value to the diagonal.
This made it work for me and I got the same result as the original implementation run locally.

3. Added some more tests and changed to pytest

4. As described in #16
I changed the data type of the activations to float32 in the `get_predictions` function,
which saves memory for larger datasets.
renzph added a commit that referenced this pull request Apr 1, 2024
@renzph
Multiple changes are part of this commit.
57f5d68 
1. Changes to `get_one_hot`
Problems are given in:
- #14
- #17
- #13

I discarded the changes in the PRs and and added more comprehensive handling of the input data in the
`SmilesDataset` class and the `get_one_hot` function.

2. Imaginary components
Frechet distance calculation fails to work for some cases because of badly conditioned matrices,
as described here #15.

Could not reproduce the error locally, but could do so on colab.

Fixed it in `calculate_frechet_distance` by checking if the first `covmean` computation  is real add a small value to the diagonal.
This made it work for me and I got the same result as the original implementation run locally.

3. Added some more tests and changed to pytest

4. As described in #16
I changed the data type of the activations to float32 in the `get_predictions` function,
which saves memory for larger datasets.
renzph added a commit that referenced this pull request Apr 1, 2024
@renzph
Multiple changes are part of this commit.
ad2fb7a 
1. Changes to `get_one_hot`
Problems are given in:
- #14
- #17
- #13

I discarded the changes in the PRs and and added more comprehensive handling of the input data in the
`SmilesDataset` class and the `get_one_hot` function.

2. Imaginary components
Frechet distance calculation fails to work for some cases because of badly conditioned matrices,
as described here #15.

Could not reproduce the error locally, but could do so on colab.

Fixed it in `calculate_frechet_distance` by checking if the first `covmean` computation  is real add a small value to the diagonal.
This made it work for me and I got the same result as the original implementation run locally.

3. Added some more tests and changed to pytest

4. As described in #16
I changed the data type of the activations to float32 in the `get_predictions` function,
which saves memory for larger datasets.
renzph added a commit that referenced this pull request Apr 1, 2024
@renzph
Multiple changes are part of this commit.
4166328 
1. Changes to `get_one_hot`
Problems are given in:
- #14
- #17
- #13

I discarded the changes in the PRs and and added more comprehensive handling of the input data in the
`SmilesDataset` class and the `get_one_hot` function.

2. Imaginary components
Frechet distance calculation fails to work for some cases because of badly conditioned matrices,
as described here #15.

Could not reproduce the error locally, but could do so on colab.

Fixed it in `calculate_frechet_distance` by checking if the first `covmean` computation  is real add a small value to the diagonal.
This made it work for me and I got the same result as the original implementation run locally.

3. Added some more tests and changed to pytest

4. As described in #16
I changed the data type of the activations to float32 in the `get_predictions` function,
which saves memory for larger datasets.
renzph added a commit that referenced this pull request Apr 1, 2024
@renzph
Add multiple fixes
bf69345 
1. Changes to `get_one_hot`
Problems are given in:
- #14
- #17
- #13

I discarded the changes in the PRs and and added more comprehensive handling of the input data in the
`SmilesDataset` class and the `get_one_hot` function.

2. Imaginary components
Frechet distance calculation fails to work for some cases because of badly conditioned matrices,
as described here #15.

Could not reproduce the error locally, but could do so on colab.

Fixed it in `calculate_frechet_distance` by checking if the first `covmean` computation  is real add a small value to the diagonal.
This made it work for me and I got the same result as the original implementation run locally.

3. Added some more tests and changed to pytest

4. As described in #16
I changed the data type of the activations to float32 in the `get_predictions` function,
which saves memory for larger datasets.

5. Change to pyproject.toml
@renzph renzph force-pushed the master branch 2 times, most recently from 53a08c2 to f806d58 Compare April 1, 2024 15:55
@renzph
Copy link
Member

renzph commented Apr 1, 2024

Hey Stephan. Thank you so much for your input.

I changed this in the new version at

FCD/fcd/fcd.py

Line 79 in f806d58

model(batch.transpose(1, 2).float().to(device)).to("cpu").detach().numpy().astype(np.float32)

@renzph renzph closed this Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hogru @renzph