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

Make Python PS torch compatible #220

Merged
merged 4 commits into from
Aug 28, 2023
Merged

Make Python PS torch compatible #220

merged 4 commits into from
Aug 28, 2023

Conversation

PicoCentauri
Copy link
Collaborator

@PicoCentauri PicoCentauri commented Aug 17, 2023


📚 Documentation preview 📚: https://rascaline--220.org.readthedocs.build/en/220/

@github-actions
Copy link

github-actions bot commented Aug 17, 2023

Here is a pre-built version of the code in this pull request: wheels.zip, you can install it locally by unzipping wheels.zip and using pip to install the file matching your system

@PicoCentauri PicoCentauri force-pushed the ps_torch branch 4 times, most recently from ef988c2 to 1aaa9ff Compare August 18, 2023 07:23
@PicoCentauri PicoCentauri marked this pull request as ready for review August 18, 2023 07:24
Copy link
Owner

@Luthaf Luthaf left a comment

Choose a reason for hiding this comment

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

A bit more cleanup and this should be good to go!

@PicoCentauri
Copy link
Collaborator Author

Thanks for the cleanup!

Co-authored-by: Guillaume Fraux <[email protected]>
@PicoCentauri
Copy link
Collaborator Author

I wonder why torch was complaining about the change of the forward function. The compiler was telling

E           Unknown type name 'CalculatorBase':
E             File "/Users/philiploche/repos/rascaline/.tox/torch-tests/lib/python3.11/site-packages/rascaline/utils/power_spectrum/calculator.py", line 12
E               def __init__(
E                   self,
E                   calculator_1: CalculatorBase,
E                                 ~~~~~~~~~~~~~~ <--- HERE
E                   calculator_2: Optional[CalculatorBase] = None,
E               ):
E           'PowerSpectrum.__init__' is being compiled since it was called from '__torch__.rascaline.torch.utils.power_spectrum.calculator.PowerSpectrum'

But he already had compiled the non-torch Power Spectrum version...

@Luthaf
Copy link
Owner

Luthaf commented Aug 28, 2023

This also required some changes in time-graph to make valgrind happy: Luthaf/time-graph@eabad48

@Luthaf Luthaf merged commit 64d177e into master Aug 28, 2023
21 checks passed
@Luthaf Luthaf deleted the ps_torch branch August 28, 2023 13:37
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.

2 participants