Make a minimal dependency package for MDF

[pgleeson]

It should be possible to install MDF for python with minimal dependencies (just enough to build, save and run with exec engine models).

Possibly PyTorch (or Tensorflow) would be the one large package to be required, but PsyNeuLink etc. should be optional extras.

[sakshikaushik717]

Can I work on this? I am an Outreachy applicant and might need some help here but I have enough experience in Python so this should be an interesting task for me.

[pgleeson]

Thanks @sakshikaushik717. As discussed with @davidt0x, what we'd like is that if a user types pip install . that they just get the minimal set of packages needed to build and run MDF, but if they type pip install .[dev] they get all the extra packages related to the export/import targets such as NeuroML, PsyNeuLink, etc.

This would involve updating https://github.com/ModECI/MDF/blob/main/setup.py, possibly moving some of the info from there to https://github.com/ModECI/MDF/blob/main/setup.cfg, and specifying which packages are extra requirements for dev, which would probably be those below the line here:

MDF/setup.cfg

Line 48 in 00957e7

NeuroMLlite<0.5.0

.

The Github action file https://github.com/ModECI/MDF/blob/main/.github/workflows/ci.yml would have to be updated to use dev, and a new workflow file set up which just installs the minimal set of dependencies and tests some of the examples.

[davidt0x]

For now I think we need to keep the extra requirements in setup.py. This is because we are defining all as a catch all programmatically. I don't know if there is a clean way to do that with the setup.cfg syntax.

I think the first step for this task is to go through the test suite and figure out which tests are core MDF tests that need to pass. My initial reaction is that anything under these modules should pass with "core" mdf:

tests/test_model.py
tests/test_onnx_functions.py
tests/test_scheduler.py
tests/test_mdf_functions.py
examples/MDF

Can you think of any other @pgleeson? One approach might be to create some custom pytest markers for psyneulink, neuroml, pytorch, etc. We can then go through all the tests that I didn't mention above and mark them with their appropriate marker. This can either be done at the module level or programatically based on module name. So for example, the tests\interfaces\pytorch would by marked with pytorch. The tests generated by tests/test_examples.py would need to mark tests programmatically as it collects them based on the directory they reside I suppose. For example, a script in examples/PyTorch would need to be marked with pytorch for example. You can see an example of how to do this here

Once this is done, we just need to then make sure tests pass with core MDF pytest -m "not pytorch and not psyneulink and not neuroml ...". I think we can also create a marker programatically called core that is the collection of tests that satisfy this. It might seem tedious this way but I think having tests marked for specific backends might become useful down the line. The main reason to do it this way I think is that tests not marked become a part of core by default, which I think is a good practice.

Once that is done, the goal would be to make extra requirements that correspond to these backends. The base install_requires would just need to pass the core tests. In CI I think we would still want to test everything but it would be good to modify the workflow to install in stages. So pip install . run core tests. Then pip install .[all] and run non core tests.

Finally, I think it would be best practice to modify code in the main library (and probably examples as well) that imports a backed to catch module not found errors and print a nicer error message to the user. Something like, it looks like you are trying to run PyTorch examples with MDF, make sure to pip install with [pytorch].

[salma77]

hello there! I am an Outreachy applicant and I would like to fix this issue, am I too late or can I still work on it? @pgleeson @davidt0x

[davidt0x]

Sure @salma77, please feel free to go ahead.

[parikshit14]

@davidt0x, as discussed with @pgleeson we primarily want to remove Psyneulink and NeuroML from the core installation of MDF. Read your comments above and I tried using the custom marker to mark the entire module(tests/test_examples.py) as psyneulinktest, also defined a new life that declared the custom marker (ref)

marker: pytestmark = pytest.mark.psyneulinktest

After applying this in tests/test_examples.py, tested using

python -m pytest -ra -m "not psyneulinktest" tests/test_examples.py

and this looks good as the output shows it has deselected everything(which is expected):

But when I try to uninstall psyneulink and then run the test it gives me an import error for psyneulink.

Also currently using a marking technique to mark the entire file as there is no clear distinction for which particular function is for psyneulink, so how should we mark only the function which is using psyneulink particularly

[davidt0x]

Hi @parikshit14,

Ok, so it looks like there is an import psyneulink statement at the top of test_examples.py, see here. We have two options. 1) Make this a conditional import that is only imported when PsyNeuLink tests are enabled. 2) Check with @kmantel if this check that is happening at the top is still needed at all? Some of the tests are being marked as xfail for a reason I don't understand. Do we still need this logic?

However, in general, there might be points in the code base (for example in modeci_mdf.interfaces) where we have simple import of psyneulink. neuroml, pytorch, etc. These should be wrapped in a try and except blocks that catch ModuleNotFoundError and print a note to install the required backend with pip install modeci_mdf[psyneulink], and reraise the exception. A good place for this check might be in the __init__.py of the appropriate interface sub module. For example, for pytorch, this check could occur in here

[kmantel]

The xfail is from a while ago when mdf wasn't fully supported on main branches in psyneulink (and before graph-scheduler was broken out to its own package), so I think it should be safe to remove the extra xfail settings.
One option might be to use pytest.importorskip for those example tests, as long as the CI continues to install psyneulink and test these. As far as I can tell, psyneulink is now only used to test these examples.

[parikshit14]

hi @kmantel @davidt0x, if we are using pytest.importorskip then do we need markers?! as pytest skips the file if this is encountered and results in true.(but I guess this indeed results in skipping of test for examples which should not have been skipped)

Also while using try and except block: are we targeting neuroml and psyneulink separately or should we keep it under the same umbrella with exception message like Install the required backend with pip install .[all]

[davidt0x]

I didn't know about pytest.importskip. It could be used at the top of the tests for specific interfaces. For example, it could be used here. But, I still think it is better to mark the tests. Especially, for examples we would not want to skip all the examples if psyneulink wasn't installed. I think we all need to start getting into the habit of marking our tests with a label for the backend if it is required.

I think we want to target all backends separately ideally. It would be great if someone could install a version of MDF only for PsyNeuLink and Torch, or PsyNeuLink and NeuroML, depending on what they were interested in.

[kmantel]

importorskip might require some reorganization, yeah. Marking is a good idea in general, but I'm not sure if there's also a way to automatically skip a certain marker if it's not installed.

I think we could at least split up test_example into multiple individual tests and mark those. For example something like

class TestExamples:
    
    @pytest.mark.parametrize(<psyneulink example tests>)
    @pytest.mark.psyneulink
    @pytest.mark.skipif(<psyneulink not installed>)
    def test_psyneulink(self):
        ...

    @pytest.mark.pytorch
    def test_pytorch(self):
        ...
    
    ...

[pgleeson]

Completed!