search

chemprop / chemprop

Message Passing Neural Networks for Molecule Property Prediction
https://chemprop.csail.mit.edu
Other
1.68k stars 565 forks source link

[v2 BUG]: `TypeError` when manually specifying `--loss-function` as its default #966

Open JacksonBurns opened 1 month ago

JacksonBurns commented 1 month ago

Describe the bug When specifying --loss-function mse via the CLI, a TypeError is raised.

Example(s) This code works:

chemprop train --output-dir chemprop_fromp \
    --data-path monomer_stability_and_heat_released.csv \
    --smiles-columns monomer \
    --target-columns G_DA_200 H_RO_200 \
    --task-type regression \
    --metrics mae mse rmse r2 \
    --batch-size 32 \
    --show-individual-scores

but this does not:

chemprop train --output-dir chemprop_fromp \
    --data-path monomer_stability_and_heat_released.csv \
    --smiles-columns monomer \
    --target-columns G_DA_200 H_RO_200 \
    --task-type regression \
    --loss-function mse \  # <---- manually specifying the default loss function
    --metrics mae mse rmse r2 \
    --batch-size 32 \
    --show-individual-scores

Expected behavior Explicitly passing the default should work the same as leaving Chemprop to determine the default.

Error Stack Trace

(chemprop_v2_live) jackson@jackson-Precision-7540:~/fastprop_fromp$ . chemprop.sh 
Wrote config file to chemprop_fromp/config.toml
Traceback (most recent call last):
  File "/home/jackson/miniconda3/envs/chemprop_v2_live/bin/chemprop", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/jackson/miniconda3/envs/chemprop_v2_live/lib/python3.11/site-packages/chemprop/cli/main.py", line 80, in main
    func(args)
  File "/home/jackson/miniconda3/envs/chemprop_v2_live/lib/python3.11/site-packages/chemprop/cli/train.py", line 74, in func
    main(args)
  File "/home/jackson/miniconda3/envs/chemprop_v2_live/lib/python3.11/site-packages/chemprop/cli/train.py", line 1007, in main
    train_model(
  File "/home/jackson/miniconda3/envs/chemprop_v2_live/lib/python3.11/site-packages/chemprop/cli/train.py", line 820, in train_model
    model = build_model(args, train_loader.dataset, output_transform, input_transforms)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jackson/miniconda3/envs/chemprop_v2_live/lib/python3.11/site-packages/chemprop/cli/train.py", line 744, in build_model
    criterion = Factory.build(
                ^^^^^^^^^^^^^^
  File "/home/jackson/miniconda3/envs/chemprop_v2_live/lib/python3.11/site-packages/chemprop/utils/registry.py", line 46, in build
    return clz_T(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/jackson/miniconda3/envs/chemprop_v2_live/lib/python3.11/site-packages/chemprop/nn/loss.py", line 39, in __init__
    task_weights = torch.as_tensor(task_weights, dtype=torch.float).view(1, -1)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: must be real number, not NoneType

Screenshots N/A

Environment

Checklist

Didn't this check, but I just installed from PyPI and I have no local changes.

Additional context You can download the dataset I am running this on if you want to reproduce the exact error locally from this Zenodo: https://zenodo.org/records/10892219

shihchengli commented 1 month ago

I think this is fixed in #942. In that PR, I added task_weights = torch.ones(n_tasks) if args.task_weights is None else args.task_weights to avoid passing None into the generation of the loss_function object.

JacksonBurns commented 1 month ago

That looks like it should work. Once we release 2.0.4, let's link that PR to this issue and have it automatically close. Leaving this up until then so that people know it is fixed on main.