[Improve] Rename model parameters to be like in the paper

[GeorgeBatch]

Dear authors,

I reviewed your code and the paper to understand how the model works. I noticed that there were certain inconsistencies in how you named the dimensions in the network architectures. I think it would be helpful to have them named in the same way in both the code and the paper. I provide the mapping in the table below:

Description	Code	Paper
Bag size	N	K
Attention in_features	L	M
Matrix V	D(out) x L(in)	L(out) x M(in)
Vector w	K(out) x D(in)	L(in) x 1(out)
Attention internal_features	D	L
Attention Branches (out_features)	K	1
Weighted features	Matrix M	Vector Z

Therefore, I made the following renamings:

• K -> ATTENTION_BRANCHES (parameter)
• N -> K (parameter)
• M -> Z (matrix)
• L -> M (parameter)
• D -> L (parameter)

Testing:

I tested the original and the new models (see screenshots). I created 4 model instances (original code, renamed code) x (Attention, Gated Attention). Then I tested that for the same architecture, given a fixed random seed:

1. The weights tensors are the same for the original and the renamed models
2. The output tensors are the same for the original and the renamed models

I think merging this pull request will make the code and the paper notation much more consistent and allow people to understand it much faster.

Kind regards, George Batchkala

---

Weights check

---

Attention outputs check

---

Gated attention outputs check

[GeorgeBatch]

Hi @Borda, thanks for approving the changes! I do not have permission to merge the pull request.

[Borda]

Hi @Borda, thanks for approving the changes! I do not have permission to merge the pull request.

True, this repo seems to be stale so I have forked it and planing to integrate all these fixes and improvements... Would you be interested in sending quick PR there?

[max-ilse]

I can do it later today :)

Op vr 5 jan. 2024 om 18:18 schreef Jirka Borovec @.***>

Hi @Borda https://github.com/Borda, thanks for approving the changes! I do not have permission to merge the pull request.

True, this repo seems to be stale so I have forked it and planing to integrate all these fixes and improvements... Would you be interested in sending quick PR there?

— Reply to this email directly, view it on GitHub https://github.com/AMLab-Amsterdam/AttentionDeepMIL/pull/28#issuecomment-1879010564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNXTIRZ7KS56WHFPR7QRYTYNAYVPAVCNFSM6AAAAAAY5WOGFSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZZGAYTANJWGQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[GeorgeBatch]

I’m happy to contribute to either this or the forked repository.

@Borda @max-ilse, I think updating the original repository will have more visibility. It made sense to me when I was looking for the code of the original AB-MIL paper that the official implementation and the paper itself have the same authors and come from the same group - AMLab-Amsterdam. On arXive there is also an option to add the repository link, which will make it even easier to find.

[Borda]

I think updating the original repository will have more visibility.

I would be more then happy to keep all in the main repo but it seems stále... =(

[GeorgeBatch]

I have never dealt with stale branches, so I do not really understand what to do in this situation.

I think my rename-like-in-paper branch might be stale (https://github.com/GeorgeBatch/AttentionDeepMIL/branches/stale) But the main branch in this repository does not seem to be stale (https://github.com/AMLab-Amsterdam/AttentionDeepMIL/branches/stale)

[Borda]

It is not stále branch but abandoned repo

[GeorgeBatch]

@Borda, I got confused because you approved the pull request. I thought you had the write access to this repository, so I did not see the point in moving away from this repository.

@max-ilse, will you be interested in updating this repository?

[max-ilse]

I have to check if I still have access. Will let you know!

Op ma 8 jan. 2024 om 14:18 schreef George Batchkala < @.***>

@Borda https://github.com/Borda, I got confused because you approved the pull request. I thought you had the write access to this repository, so I did not see the point in moving away from this repository.

@max-ilse https://github.com/max-ilse, will you be interested in updating this repository?

— Reply to this email directly, view it on GitHub https://github.com/AMLab-Amsterdam/AttentionDeepMIL/pull/28#issuecomment-1880990822, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNXTIXZBL36YWRD25KQNM3YNPWYZAVCNFSM6AAAAAAY5WOGFSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBQHE4TAOBSGI . You are receiving this because you were mentioned.Message ID: @.***>

[max-ilse]

Thanks so much for the PR!