Skip to content

Refactoring, adding tests and installing instructions#23

Open
garciadias wants to merge 14 commits intoibrahimethemhamamci:mainfrom
garciadias:Refactoring
Open

Refactoring, adding tests and installing instructions#23
garciadias wants to merge 14 commits intoibrahimethemhamamci:mainfrom
garciadias:Refactoring

Conversation

@garciadias
Copy link

@garciadias garciadias commented Apr 10, 2025

Hello Ibrahim,

I am an ML engineer at King's College London. I am working on building Visual Language models on MRI, and your work is very similar to the type of architecture I am trying to create.
Congratulations on your article, and thank you for sharing the code.

Since I will use your code as a baseline comparison and inspiration for my development, I decided to clean and refactor the code. I am sharing here a PR to include tests, python environment settings and docker instructions to easily build and run the code.

For this PR, I tried not to change the code in any significant way. There were some undefined variables and errors that I had to fix in order to run the tests, but they are very minimal.

Here you can see the output of running the tests:

======================================================================================= test session starts ========================================================================================
platform linux -- Python 3.11.11, pytest-8.3.5, pluggy-1.5.0
rootdir: /data/rgd/CT2Rep
configfile: pyproject.toml
plugins: cov-6.1.1, torchtyping-0.1.5, typeguard-2.13.3
collected 126 items                                                                                                                                                                                

tests/CT2Rep/ctvit/ctvit/test_attention.py .........................                                                                                                                         [ 19%]
tests/CT2Rep/ctvit/ctvit/test_ctvit.py ...........s.......ss...ssss.ss                                                                                                                       [ 44%]
tests/CT2Rep/ctvit/ctvit/test_t5.py ..........s..                                                                                                                                            [ 54%]
tests/CT2Rep/modules/test_metrics.py .......                                                                                                                                                 [ 60%]
tests/CT2Rep/modules/test_tokenizers.py ...........ss..                                                                                                                                      [ 72%]
tests/CT2Rep/modules/test_training.py ....s.....                                                                                                                                             [ 80%]
tests/CT2Rep/modules/test_utils.py .....................                                                                                                                                     [ 96%]
tests/CT2Rep/test_main.py ....                                                                                                                                                               [100%]

========================================================================================= warnings summary =========================================================================================
tests/CT2Rep/modules/test_training.py::test_train_epoch
  /data/rgd/CT2Rep/.venv/lib/python3.11/site-packages/torch/optim/lr_scheduler.py:136: UserWarning: Detected call of `lr_scheduler.step()` before `optimizer.step()`. In PyTorch 1.1.0 and later, you should call them in the opposite order: `optimizer.step()` before `lr_scheduler.step()`.  Failure to do this will result in PyTorch skipping the first value of the learning rate schedule. See more details at https://pytorch.org/docs/stable/optim.html#how-to-adjust-learning-rate
    warnings.warn("Detected call of `lr_scheduler.step()` before `optimizer.step()`. "

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================================================ 113 passed, 13 skipped, 1 warning in 3.73s ============================================================================

============================= Coverage Report =======================================
Name                                            Stmts   Miss  Cover
-------------------------------------------------------------------
src/ct2rep/CT2Rep/main.py                          77     13    83%
src/ct2rep/CT2Rep/models/ct2rep.py                 26     17    35%
src/ct2rep/CT2Rep/modules/att_model.py            215    197     8%
src/ct2rep/CT2Rep/modules/caption_model.py        222    211     5%
src/ct2rep/CT2Rep/modules/data_ct.py               90     73    19%
src/ct2rep/CT2Rep/modules/dataloaders.py           26     19    27%
src/ct2rep/CT2Rep/modules/encoder_decoder.py      277    212    23%
src/ct2rep/CT2Rep/modules/loss.py                  15      9    40%
src/ct2rep/CT2Rep/modules/metrics.py               14      3    79%
src/ct2rep/CT2Rep/modules/optimizers.py             9      6    33%
src/ct2rep/CT2Rep/modules/tokenizers.py            75      3    96%
src/ct2rep/CT2Rep/modules/trainer.py              150     13    91%
src/ct2rep/CT2Rep/modules/utils.py                 32      0   100%
src/ct2rep/CT2Rep/modules/visual_extractor.py      16     11    31%
src/ct2rep/ctvit/ctvit/attention.py               181      1    99%
src/ct2rep/ctvit/ctvit/ctvit.py                   323    175    46%
src/ct2rep/ctvit/ctvit/t5.py                       54      1    98%
-------------------------------------------------------------------
TOTAL                                            1802    964    47%

If you are happy to collaborate, I would keep sending PRs to improve the code, eliminate duplications and improve test coverage.

I am also a MONAI collaborator, so I would happily help you get your model listed on MONAI bundles.

Many thanks,

Rafael

Signed-off-by: R. Garcia-Dias <[email protected]>
Signed-off-by: R. Garcia-Dias <[email protected]>
Signed-off-by: R. Garcia-Dias <[email protected]>
Signed-off-by: R. Garcia-Dias <[email protected]>
Signed-off-by: R. Garcia-Dias <[email protected]>
Signed-off-by: R. Garcia-Dias <[email protected]>
Signed-off-by: R. Garcia-Dias <[email protected]>
…djust padding values, and streamline function definitions.

Signed-off-by: R. Garcia-Dias <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant