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

Problems with dependencies #51

Open
stefhk3 opened this issue Aug 16, 2023 · 2 comments
Open

Problems with dependencies #51

stefhk3 opened this issue Aug 16, 2023 · 2 comments

Comments

@stefhk3
Copy link
Collaborator

stefhk3 commented Aug 16, 2023

If we want to make this a broad NMR kit, we might get into issues with dependencies and their versions. Right now, I try to add a prediction model which was done previously. If I try to run it with a set of current software versions, I get an error, which seems to be due to a change in RDkit:
conformer = mol.GetConformer()[conf_i]
needs to become
conformer = mol.GetConformer(conf_i.item())
conf_i is a numpy type, the item() converts to int.
This workedin rdkit 2020.09.1.0
If I fix this, I get another error - the error comes from the code I want to import, and it is a wrong number of elements in a list. Whilst this is not directly due to versions, it does work in a different combination of library versions.
I think we will get into this issue more frequently - if we want to use existing code. Ideally, we would isolate the components. I am not an expert on docker, but can't docker do that (if we set it up)?

@CS76
Copy link
Member

CS76 commented Aug 19, 2023

Hello @stefhk3 thanks for pointing this out. I did think about this situation while designing this and our current solution is meant to avoid situations like this in its first place….
let’s say we resolve this issue in the current version and distribute the docker image you can always guarantee that it works on certain docker image release.

Here are some possible solutions

  1. In a situation like this where we are packaging several components neither docker nor venv will be able to solve the problem because different components require different versions. It needs to be packaged as standalone docker images and call them explicitly from FASTAPI container and internally track the progress on another container to fetch and return the result of the run.
  2. Other work around would be to install these packages manually and let the dependencies call them via aliases which is not advisable because we need to draw a limit otherwise this becomes non maintainable.

For now I still think we need to update the code to work with latest RDKit instead of the workarounds or complicating dependencies on multiple containers.

@CS76
Copy link
Member

CS76 commented Aug 19, 2023

Do you already have a docker image with your model and RDKit running?. If so share the details here,
we can then run docker exec and run this in a separate docker image and be able to isolate everything.

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

No branches or pull requests

2 participants