Biobix / DeepRibo

GNU General Public License v3.0
13 stars 3 forks source link

deepribo v1.1 unexpected behaviour + minor issue #7

Closed RickGelhausen closed 4 years ago

RickGelhausen commented 4 years ago

Hi @jdcla,

thank you again for tagging the new release. I downloaded it and installed the dependencies using your requirements.yaml. When I tested it on some test dataset, I ran into a minor problem, which was probably caused by the updated pandas dependency.

In the CustomLoader, within DeepRibo.py, I encounter the following error message:

Creating predictions using model deepribo/DeepRibo_model_v1.pt
Using args Namespace(COV_motifs=32, FC_nodes=[1024, 512], GPU=False, GRU_bidirect=True, GRU_layers=2, GRU_nodes=128, coverage=[0.095593], data_path='deepribo/', dest='deepribo/TruDWT-1/predictions.csv', model='deepribo/DeepRibo_model_v1.pt', model_type='CNNRNN', num_workers=10, pred_data='TruDWT-1/', rpkm=0.166208542327924, verbose=False)
Traceback (most recent call last):
  File "DeepRibo-1.1/src/DeepRibo.py", line 588, in <module>
    sys.exit(ParseArgs())
  File "DeepRibo-1.1/src/DeepRibo.py", line 448, in __init__
    getattr(self, args.command)()
  File "DeepRibo-1.1/src/DeepRibo.py", line 579, in predict
    predict(args.data_path, [args.pred_data],
  File "DeepRibo-1.1/src/DeepRibo.py", line 411, in predict
    pred_loader = loadDatabase(data_path, pred_data, pred_cutoff, batch_size,
  File "DeepRibo-1.1/src/DeepRibo.py", line 254, in loadDatabase
    data = CustomLoader(data_path, data, cutoff)
  File "DeepRibo-1.1/src/DeepRibo.py", line 65, in __init__
    cond_2 = tmp_s_df["coverage_elo"] >= x[2]
  File "/home/ubuntu/miniconda3/envs/deepribo/lib/python3.8/site-packages/pandas/core/ops/common.py", line 64, in new_method
    return method(self, other)
  File "/home/ubuntu/miniconda3/envs/deepribo/lib/python3.8/site-packages/pandas/core/ops/__init__.py", line 526, in wrapper
    res_values = comparison_op(lvalues, rvalues, op)
  File "/home/ubuntu/miniconda3/envs/deepribo/lib/python3.8/site-packages/pandas/core/ops/array_ops.py", line 234, in comparison_op
    raise ValueError("Lengths must match to compare")
ValueError: Lengths must match to compare

when running the following command (the values for rpkm and coverage comming from the s_curve_cutoff_estimation.R script):

python3 DeepRibo-1.1/src/DeepRibo.py predict deepribo/ --pred_data TruDWT-1/ -r 0.166208542327924 -c 0.095593 --model deepribo/DeepRibo_model_v1.pt --dest deepribo/TruDWT-1/predictions.csv --num_workers 10`

I was able to pinpoint this to an issue that the coverage is somehow saved as a list, rather than a single value, this crashes:

cond_2 = tmp_s_df["coverage_elo"] >= x[2]

where a value is expected but a list is given.

Is there any reason why the argparse nargs=+ option is used to store the coverage parameter? As this causes it to be stored as a list?

            parser.add_argument('-c', '--coverage', nargs='+', type=float,
                                required=True, help="minimum cutoff of "
                                "coverage value to filter the data used for "
                                "predictions order")

When I change this locally, the prediction works.

Another very small thing. Is there any reason why deepribo is called using a sys.exit() command? As this is an error call.

if __name__ == "__main__":
    sys.exit(ParseArgs())

This does not change anything when you run the code locally, as the results are written to file and then deepribo "crashes". When using a workflow management system however, this will cause deepribo to crash the entire workflow when it finishes the prediction/training call.

If there is no reason that I am overlooking, you could simply use:

if __name__ == "__main__":
    ParseArgs()

Thank you for your help.

jdcla commented 4 years ago

@RickGelhausen Thank you for the report. As you pointed at there is no reason for multiple arguments in the predict function of DeepRibo. I have no idea how this has not resulted in any errors for me before. It is fixed now.

I have cleaned up the sys.exit() arguments. They must have been an artifact from when I started creating these files that I have never really thought about.

I hope everything works as expected.

EDIT: I have retagged v1.1 to include the fix.

RickGelhausen commented 4 years ago

Thank you for fixing this so quickly and for creating this useful tool.