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

fix: OSError when retrieving source code for lambda functions #606

Merged
merged 5 commits into from
Jun 6, 2024

Conversation

gcroci2
Copy link
Collaborator

@gcroci2 gcroci2 commented May 30, 2024

While training a big network I got: OSError: could not get source code from the _save_model() method of the Trainer class, in particular when we do str_expr = inspect.getsource(key["transform"]). The error indicates that the inspect module is unable to retrieve the source code for the lambda function stored in key["transform"].

  • I fixed it by using dill to serialize and deserialize the functions, which appears to be more reliable.
  • I changed the scipy.signal.bspline import to scipy.interpolate.BSpline to be up to date with a more recent version of scipy (minimum 1.13.1) as indicated in one of their latest releases (1.13.0). I did this since we prefer to not fix the versions, but just to indicate the minimum, and the previous requirement on scipy made the tests break because of this expired deprecation.

@coveralls
Copy link

coveralls commented May 30, 2024

Coverage Status

coverage: 83.933% (-0.1%) from 84.071%
when pulling 3183e41 on fix_inspect_function_trainer_gcroci2
into e422fb3 on main.

@gcroci2 gcroci2 requested a review from DaniBodor May 31, 2024 09:01
Copy link
Collaborator

@DaniBodor DaniBodor left a comment

Choose a reason for hiding this comment

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

  • I fixed it by using dill to serialize and deserialize the functions, which appears to be more reliable.

It's not completely clear to me what this does, but if it works now, I'm happy to accept the change.

  • I changed the scipy.signal.bspline import to scipy.interpolate.BSpline to be up to date with a more recent version of scipy (minimum 1.13.1) as indicated in one of their latest releases (1.13.0). I did this since we prefer to not fix the versions, but just to indicate the minimum, and the previous requirement on scipy made the tests break because of this expired deprecation.

Given that we have much less time for this project now, maybe we can discuss fixing versions of many packages to avoid similar problems in the future? Also given that development of this package will slow down, at least for the moment, and current package versions work well with what we are currently doing, it may be less important to stay up to date with newest package versions.
This would be a separate PR though.

@gcroci2
Copy link
Collaborator Author

gcroci2 commented Jun 6, 2024

  • I fixed it by using dill to serialize and deserialize the functions, which appears to be more reliable.

It's not completely clear to me what this does, but if it works now, I'm happy to accept the change.

  • inspect.getsource (old way) retrieves the source code of the function or lambda expression passed to it.
    It looks up the function definition in the current environment. Lambda functions are often anonymous and defined inline, making it challenging for inspect to locate their source code reliably, especially if they're not defined in a module or a directly accessible script. When I run the code on Snellius (I get the error only in that case), it's very likely that the exact location or the context where the lambda was defined might not be accessible or might not be present in the expected form.
  • With the new way, the serialization (dill.dumps) serializes the lambda function into a byte stream. The deserialization (dill.loads) converts the byte stream back into the original function object. This means that even if the function's source code is not directly accessible, dill (that captures the complete context of the function) can reconstruct it from the serialized data. The deserialized function retains the context and source information (captured in the serialized data) that inspect can then access.

I hope it's clearer but in case we can have a chat about this :)

Given that we have much less time for this project now, maybe we can discuss fixing versions of many packages to avoid similar problems in the future? Also given that development of this package will slow down, at least for the moment, and current package versions work well with what we are currently doing, it may be less important to stay up to date with newest package versions. This would be a separate PR though.

Very good point, I opened issue #607 :)

@gcroci2 gcroci2 merged commit 655f301 into main Jun 6, 2024
5 checks passed
@gcroci2 gcroci2 deleted the fix_inspect_function_trainer_gcroci2 branch June 6, 2024 15:06
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