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

Modification of pyrms import when debugging #2363

Closed
calvinp0 opened this issue Jan 26, 2023 · 4 comments
Closed

Modification of pyrms import when debugging #2363

calvinp0 opened this issue Jan 26, 2023 · 4 comments
Labels
stale stale issue/PR as determined by actions bot

Comments

@calvinp0
Copy link
Member

calvinp0 commented Jan 26, 2023

Motivation or Problem

When VSCode/PyCharm are being used to debug RMG-Py, the execution of the code during the debug process will suddenly stop without reporting the exception. The exception will only be reported if the user ticks the 'User Uncaught Exceptions'/'Raise Exceptions' in the 'BREAKPOINTS' section. The issue is that during the debugging process there is a few lines of code it fails

File: reactors.py

try:
    from pyrms import rms
    from diffeqpy import de
    from julia import Main
except:
    pass

The reported error is 'ModuleNotFoundError' for 'pyrms'

Desired Solution

This error can be rectified a couple of ways:

  1. Modify the except statement to include 'ModuleNotFoundError'
  2. Check if in debug mode
import sys

gettrace = getattr(sys, 'gettrace', None)

if gettrace is None:
    try:
        from pyrms import rms
        from diffeqpy import de
        from julia import Main
   except:
        pass
elif gettrace():
        ###Run Alternative pyrms/diffeqpy load as explained in point 3
  1. Import pyrms and diffeqpy modification
elif gettrace():
        from julia.api import Julia
        jl=Julia(compiled_modules=False)
        from pyrms import rms
        from diffeqpy import de
        from julia import Main
  1. Improve the loading of RMS - PyJulia has the ability to create a system image as *.sys file. This will improve the load time of RMS. However, it will require updating PyJulia to version 0.6.0 (right now RMG uses PyJulia - 0.5.3)
python -m julia.sysimage sys.so

Now assuming the system image, named sys.so (can be changed of course), has been created - then changes to importing Jula from julia.api will be required.

        from julia.api import Julia
        jl=Julia(sysimage="sys.so") #This may need modification to also include the path
  1. If point four is followed through, then creating of the system image can be part of the installation of RMG-Py for the Source instructions.

Full Solution

import sys
gettrace=getattr(sys, 'gettrace', None)

if gettrace is None:
    try:
        from pyrms import rms
        from diffeqpy import de
        from julia import Main
    except ModuleNotFoundError:
        pass
elif gettrace():
    try:
        from os.path import dirname, abspath, join
        path_rms = dirname(dirname(dirname(abspath(__file__))))
        from julia.api import Julia
        jl = Julia(sysimage=join(path_rms,"sys.so"))
        from pyrms import rms
        from diffeqpy import de
        from julia import Main
    except:
        from julia.api import Julia
        jl = Julia(compiled_modules=False)
        from pyrms import rms
        from diffeqpy import de
        from julia import Main
@JacksonBurns
Copy link
Contributor

JacksonBurns commented Jan 26, 2023

I have a pull request open right now that is semi-related to this issues (#2359) because the ignored import-related exception is not best practice. A couple notes on this:

Modify the except statement to include 'ModuleNotFoundError'

I think that the exception that is actually getting caught here is not always a ModuleNotFoundError - that exception is raised later in the execution of reactors.py when it attempts to use pyrms but this import silently failed. The reason that the try/except was added was because of this error that crops up with nosetests.

Check if in debug mode

I would be curious to see how this interacts with the nosetests execution.

As far as the full solution goes, I think it would be better to fix the underlying issue (nosetests no playing well with Julia, forcing us to do this try/except-pass) rather than write this on top of it. If we were to (somehow) remove the try/except (aka make nosetests and Julia work rogether), or get it to only ignore exceptions when in nosetests, would it fix the issue with the debugging?

@calvinp0
Copy link
Member Author

Hi @JacksonBurns. Your PR doesn't really deal with the core of this ticket's issue. Yes, there is not always a ModuleNotFoundError. ModuleNotFoundError only occurs when the user is running RMG in debug mode (aka calling python and not python-jl). Your PR, as you wrote in the output message, is an installation issue that has occured. So my ticket is under the assumption the user has set everything up correctly. Sure, you could combine the ModuleNotFoundError and the julia.core.UnsupportedPythonError if you want.

Furthermore, my issue isn't related to running a unittest. I am not running unittests and I assume if a person is running a test they should have Julia/PyJulia/PyCall/RMS all correctly configured. I am literally running my IDE in debug mode to try catch another error. Also, as I mentioned, I believe creating a system image of RMS would be beneficial, even just for debugging because it saves an enormous amount of time. However, please note, if RMS is compiled into a system image and then in the far future someone decides to update RMS, the compiled system image will need to be recompiled.

@mjohnson541
Copy link
Contributor

When I was originally working with pyjulia it was quite difficult to build those system images...but it looks a lot easier now. It seems to me that we could potentially update the system images during the make command and make it operate just like cython, but I'm not quite sure what nuances might come up. Other than running the build, which should be straightforward, I don't tihnk updating pyjulia to 0.6 would have any complications

@github-actions
Copy link

This issue is being automatically marked as stale because it has not received any interaction in the last 90 days. Please leave a comment if this is still a relevant issue, otherwise it will automatically be closed in 30 days.

@github-actions github-actions bot added the stale stale issue/PR as determined by actions bot label Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale stale issue/PR as determined by actions bot
Projects
None yet
Development

No branches or pull requests

3 participants