-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Hi, I'm currently working on multi-objective biological sequence design problems. I appreciate you sharing the implementation of MOGFN-AL!
I found two issues in your code as follows.
-
mogfn_seq.py#L547, mogfn_seq.py#L572
pos_logits[:, lens+1] = -1000 # can't change last token
pos_logits_temp[:, lens+1] = -1000 # can't change last token
These lines might prevent the modification of non-last tokens if lens contains more than one distinct value. I think that these lines should be changed into
pos_logits[np.arange(len(lens)), lens+1] = -1000 # can't change last token
pos_logits_temp[np.arange(len(lens)), lens+1] = -1000 # can't change last token -
mogfn_seq.py#526-L635
command:
python scripts/black_box_opt.py optimizer=mogfn_seq task=regex tokenizer=protein optimizer.encoder_obj=mlm surrogate=multi_task_exact_gp acquisition=nehvi optimizer.use_acqf=True optimizer.pref_cond=False optimizer.num_opt_steps=1500 optimizer.max_len=30 optimizer.sample_beta=32 optimizer.beta_sched=1 optimizer.pref_alpha=1 wandb_mode=disabled
I slightly changed the code to utilizedtype=torch.doubleand I ran the command on the machine with RTX3090 GPU.
Most of the trials had no problem, but some of the trials were stuck at mogfn_seq.py#L585. This error occured becausepos_logitshad values near-1000for the problematic trials. I think that it is better to change-1000into much larger value like-1e6for better stability of the code.
- Minor issue about
max_len.
mogfn_seq.py#L94 and
mogfn_seq.py#L538 seem to do the same thing twice. I think that it is better to remove one of the-2.
I would greatly appreciate your input to validate my concerns. Thanks!