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

use the juliacall package instead of julia #394

Merged
merged 10 commits into from
May 15, 2024
Merged

use the juliacall package instead of julia #394

merged 10 commits into from
May 15, 2024

Conversation

timmysilv
Copy link
Collaborator

@timmysilv timmysilv commented May 6, 2024

Context:
The julia python package is backed by PyCall, and PyCall kinda sucks. juliacall/pythoncall is a much easier package to work with, with very simple single-file configuration

Description of the Change:
Replace uses of the julia python package with juliacall and do a bit of re-naming/tidying.

Benefits:

  • juliacall requires much less configuration up-front
  • if the required julia version from the config (~1.9.3 today) can't be found, it will automatically install a valid one at runtime using juliaup. Then, if the julia project hasn't been initialized (equivalent to the Pkg.instantiate("julia_pkg") thing we were doing before), it will do that automatically as well!
  • tl;dr the user need not do any real installation stuff anymore

Possible Drawbacks:

  • I do not know if performance is worse. It might even be better? If a reviewer could help me prove this, that would be fantastic. That said, the creator of PyCall himself seems to like PythonCall. @ziofil sent me a pretty simple notebook to use to confirm things, and the results suggest that the performance is basically the same, but we might save a smidge with some import overhead, idk.
  • I have to manually call self.astensor because for some reason, there is no auto-conversion happening from Julia to Python although it should theoretically happen with juliacall. See note on memory below.

Notes on memory consumption
Julia arrays returned to Python are wrapped in a Python object called an ArrayValue. According to the FAQ, the object satisfies the Python array protocol, but is not actually a NumPy array. If needed (it sometimes is), the class has a to_numpy function, and its default value for copy is True. If we don't intend on mutating array values, we can set this to False and just read the values from Julia's allocated memory. The performance trade-offs would require extensive benchmarking to be sure which is better.

Copy link

codecov bot commented May 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.43%. Comparing base (3afde75) to head (8517f2a).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #394      +/-   ##
===========================================
- Coverage    87.43%   87.43%   -0.01%     
===========================================
  Files           81       81              
  Lines         6137     6135       -2     
===========================================
- Hits          5366     5364       -2     
  Misses         771      771              
Files Coverage Δ
mrmustard/math/backend_numpy.py 100.00% <ø> (ø)
mrmustard/math/backend_tensorflow.py 100.00% <ø> (ø)
mrmustard/utils/settings.py 99.02% <100.00%> (-0.02%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3afde75...8517f2a. Read the comment docs.

Copy link
Collaborator

@ziofil ziofil left a comment

Choose a reason for hiding this comment

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

It works! 💯

@timmysilv
Copy link
Collaborator Author

I just added a hack to suppress the julia outputs that happen the first time you import it (in other words, the first time you increase the precision). If we don't mind the outputs happening in a fresh environment once, then I think we should revert that change. lmk what you think!

@timmysilv
Copy link
Collaborator Author

confirmed offline, it's ok to have the output

@timmysilv timmysilv merged commit c987bd0 into develop May 15, 2024
8 checks passed
@timmysilv timmysilv deleted the use-juliacall branch May 15, 2024 13:16
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.

None yet

2 participants