ersilia-os / ersilia

The Ersilia Model Hub, a repository of AI/ML models for infectious and neglected disease research.
https://ersilia.io
GNU General Public License v3.0
203 stars 131 forks source link

🐛 Bug: Notebook conversion in workflow #896

Closed miquelduranfrigola closed 1 week ago

miquelduranfrigola commented 10 months ago

Describe the bug.

Automated workflows are failing due to an issue in the Colab Notebook workflow: https://github.com/ersilia-os/ersilia/actions/runs/7048990424/job/19186425834

@megamanics : Do you know what may be happening here? Thanks!

Describe the steps to reproduce the behavior

No response

Expected behavior.

No response

Screenshots.

No response

Operating environment

GitHub Actions Ubuntu

Additional context

No response

megamanics commented 9 months ago

it looks like it started with this commit: https://github.com/ersilia-os/ersilia/commit/2f3c038fb2ebab81a4da5c97929287d009071e64

Image

Image

Try reverting this version to narrow down the source of the problem?

miquelduranfrigola commented 9 months ago

This is useful, thanks! @GemmaTuron do we still need levenshtein? Perhaps we have it as optional?

miquelduranfrigola commented 9 months ago

I have removed the levenshtein dependency (it was not really necessary) but unfortunately this did not solve the issue. I have decided to remove the workflow step related to the notebook working with Python API. It is somewhat redundant with the CLI step. I am unsure what was going on, in my opinion, it is related to the notebook conversion command.

GemmaTuron commented 9 months ago

Hi @miquelduranfrigola

I don't understand, the point of this action was to test the Python API command, the CLI was just an addition, so if anything the CLI is what should be removed.

GemmaTuron commented 9 months ago

I don't think the issue is with the conversion command, since this works fine for the notebook run with the CLI commands. The issue is quite simple: we need to install pandas so that the output can be parsed

try:
    import pandas as pd
except ModuleNotFoundError:
    pd = None
File ~/miniconda3/envs/ersilia/lib/python3.10/site-packages/ersilia/core/model.py:212, in ErsiliaModel._api_runner_return(self, api, input, output, batch_size)
    210 def _api_runner_return(self, api, input, output, batch_size):
    211     if output == "pandas" and pd is None:
--> 212         raise Exception
    213     if output == "json":
    214         R = []
GemmaTuron commented 9 months ago

I've tried simply by adding pandas to the installs, it works in my local but for some reason it does not in the workflow file? https://github.com/ersilia-os/ersilia/actions/runs/7260232462/workflow

miquelduranfrigola commented 9 months ago

Yes, I do not manage to solve this issue either, @GemmaTuron . For now, I have commented out the API

miquelduranfrigola commented 9 months ago

Hi @GemmaTuron - I am apparently unable to solve this issue. Should we seek help?

GemmaTuron commented 9 months ago

@miquelduranfrigola please see my comments regarding pandas in #928 If we make it a requirement of Ersilia I think the problem will be solved.

miquelduranfrigola commented 9 months ago

I am ok with having pandas as a dependency.

GemmaTuron commented 1 week ago

Hi @miquelduranfrigola

I think this is quite old and not happening any more, but not sure. Pandas is definitely no longer a requirement. @DhanshreeA have you observed this problem or we can close the issue?

miquelduranfrigola commented 1 week ago

Yes this is quite old indeed. I don't think we use pandas, but @DhanshreeA can confirm

DhanshreeA commented 1 week ago

Agree with both of you. Pandas is no longer a requirement, and this workflow no longer fails (even if it is not the best representation of what we want - but we can tackle that later). I am closing this issue for now.