iqbal-lab-org / make_prg

Code to create a PRG from a Multiple Sequence Alignment file
Other
22 stars 7 forks source link

Implements update_shared_data as a class attribute in a singleton pattern and drops Mac OS X support #46

Closed leoisl closed 1 year ago

leoisl commented 1 year ago

Sorry that this PR is now doing two things.

This PR firstly describes the implementation of update_shared_data as a class attribute in a singleton pattern. This has the benefit of sharing in-memory data efficiently between the parent and child processes when multiprocessing during make_prg update operation, without the drawbacks of using global. Due to how singleton objects work, the update integration tests now have to run on different subprocesses, so pytest-forked = "^1.4.0" has to be added to dev.dependencies. Closes #45

Secondly, it drops support for Mac OS X. There are several reasons:

  1. Our tools that depend on make_prg are mostly tested on linux, e.g. make_prg and pandora provides precompiled binaries just for linux, and sample example use these (easy to change though);
  2. pandora is just available on linux through conda;
  3. pandora does not implement multithreading in Mac OS X;
  4. next release of pandora has not been tested on Mac OS X;
  5. drprg is tested just on linux runners;
  6. See slack for more discussion on this;

This support drop can be just temporary. Now we can proceed and finish the new make_prg and pandora releases this week, enabling drprg to use these new releases. We can discuss further, and if we decide to indeed support Mac OS x, we can work on this support next week.

leoisl commented 1 year ago

Update integration tests works on linux, but does not work on mac os x... trying to understand why and fix

leoisl commented 1 year ago

Sharing singleton objects does not seem to work on os x: https://github.com/leoisl/make_prg/actions/runs/3574053921/jobs/6008850030#step:8:549

What we are getting in os x (sharing is not happening, we are recreating the data in the child process):

2022-11-29 12:43:47.314 | INFO     | make_prg.update.update_shared_data:__new__:39 - Creating SingletonUpdateSharedData
2022-11-29 12:43:47.316 | INFO     | make_prg.update.update_shared_data:__init__:55 - Initialising SingletonUpdateSharedData
2022-11-29 12:43:47.318 | INFO     | make_prg.subcommands.update:run:208 - Using 1 threads to update PRGs...
2022-11-29 12:43:49.361 | INFO     | make_prg.update.update_shared_data:__new__:39 - Creating SingletonUpdateSharedData
2022-11-29 12:43:49.361 | INFO     | make_prg.update.update_shared_data:__init__:55 - Initialising SingletonUpdateSharedData

What we are getting in linux (sharing is happening):

2022-11-29 13:38:08.282 | INFO     | make_prg.update.update_shared_data:__new__:39 - Creating SingletonUpdateSharedData
2022-11-29 13:38:08.282 | INFO     | make_prg.update.update_shared_data:__init__:55 - Initialising SingletonUpdateSharedData
2022-11-29 13:38:08.282 | INFO     | make_prg.subcommands.update:run:208 - Using 1 threads to update PRGs...
2022-11-29 13:38:08.290 | INFO     | make_prg.update.update_shared_data:__new__:43 - SingletonUpdateSharedData already created
2022-11-29 13:38:08.290 | INFO     | make_prg.update.update_shared_data:__init__:58 - SingletonUpdateSharedData already initialised
leoisl commented 1 year ago

Now that Mac OS X support has been dropped, CI works, ready for review