Nit: Looks like we use an LSTM instead of a GRU?

som-shahlab / ehr_ml

Code for doing machine learning with various EHRs

MIT License

21 stars 3 forks source link

Nit: Looks like we use an LSTM instead of a GRU? #13

Closed woffett closed 3 years ago

woffett commented 3 years ago

Since it seems like patient timeline sequences aren't that long in the first place, we'd probably reap some compute benefits by switching to nn.GRU instead.

https://github.com/som-shahlab/ehr_ml/blob/48a385fe2ebdbef655bd4c6b6dd9a73a4e3f76b4/ehr_ml/clmbr/rnn_model.py#L262

EthanSteinberg commented 3 years ago

Good point! Switching back to a GRU might be a good idea. I think I just switched to an LSTM after the paper got published since it performed slightly better, but it's pretty marginal as far as the big picture is concerned.

woffett commented 3 years ago

Switched to GRU in the latest PR.