Comments on Jupyter notebooks

[tedinburgh]

Added my comments to the notebooks (to see the comment in context, open the link and click on LoadDiff for the relevant file).

Overall I would also suggest asking for feedback from some additional people who may be users of the platform (perhaps after the next version if there is a revision after my comments).

[tedinburgh]

Cox Modelling: CoxFunction_Translated.ipynb

This looks a bit strange at the start of the notebook, is it necessary to include for the code to work?

0e7052e#r158874321

In my opinion the notation should be

$X = x_A$ and $X = x_B$

$\frac{h(t | X = x_A)}{h(t | X = x_B)}$

etc

0e7052e#r158874553

Should this mention how to test the PH assumption then?

0e7052e#r158874615

Personal preference is that acronyms in Latex maths syntax should be in regular plain-text, not as an (italic) variable

0e7052e#r158875175

I think we need to be more clear around CIs and p-values. In my opinion, the second bullet point is implying that if the CI does not include 1 then the effect may not due to chance (which is not true).

0e7052e#r158875655

As far as I understand, you can model time-varying covariates in Cox PH?

0e7052e#r158875982

My view on the structure of the notebook is that the long explanation above should be broken up with code chunks to demonstrate these concepts using examples.

Have you looked at some other tutorials or examples to help structure this? E.g. https://github.com/CamDavidsonPilon/lifelines/tree/master/examples

0e7052e#r158876242

I don't think we should explain things like pandas and numpy, I am assuming our audience already knows these.

However perhaps it's useful to give a brief overview of lifelines (and a link to the docs), as a library that is particularly relevant to this type of modelling.

0e7052e#r158877647

Personally, I don't think the purpose of the Jupyter notebook should be a line by line explanation of code. They can review the code itself for this (especially as you have done a great job in commenting your code). I think users need to understand the inputs and the outputs of the main function, and maybe some bullet points of the steps included.

0e7052e#r158878151

For anything in the notebooks, users need to be able to actually run the code. So they need to have access to any files here (either included as a file in this repository, or they know how to request this data). I don't think that's possible for this example, which I believe is SINAN data.

(this is one reason why we need synthetic data, because we can actually share it to use for teaching/training purposes).

0e7052e#r158878859

As above, I am not sure we should include the full line by line code of the function.

0e7052e#r158878947

We need to make sure any code here actually works and doesn't return an error message.

The default implementation of VERTEX (ARChetype_h5nx_synthetic_mf) has a working example of the RAPs on synthetic data if you this is useful? But we need to discuss further if there are any changes to make to the RAPs based on this implementation.

0e7052e#r158879141

[tedinburgh]

Linear Regression: GLM_Linear.ipynb

Please fix subscripts

0e7052e#r158879694

It's probably better for the CI to be generalised (rather than only specify the 95% CI). But this means the formula below should also be generalised (i.e. the Z-score instead of 1.96)

0e7052e#r158888846

I think acronyms like CI should be in regular plain-text, not as an (italic) variable

0e7052e#r158888573

I don't think we should explain well-known libraries.

0e7052e#r158888937

Please see my comments in the Cox regression Jupyter notebook about including the following in the notebook:

• line by line code

• code that cannot be run by the user because we have not shared/cannot share the data

0e7052e#r158889092

[tedinburgh]

Logistic Regression: Logistic_Regression_Doc.ipynb

Please see my comments on Cox regression and linear regression about structure, notation, audience, etc.

0e7052e#r158889599

I think we need to clearly identify the target audience, do we expect that users should be familiar with installing/importing libraries? If the answer is no, then this information is common to all notebooks, in which case perhaps we need a kind of 'getting started' notebook?

Personally I think this is too much detail about how to import a library.

0e7052e#r158889508

[tedinburgh]

Imputation / MICE: MICE_CodeDocumentation(TomData).ipynb

Please see my comments on Cox regression and linear regression about structure, notation, audience, etc.

0e7052e#r158890543

I like how this summary of the inputs/outputs is presented.

0e7052e#r158889921

I would suggest printing a smaller subset of the table, e.g. print(main_df[columns].head()) for some defined columns, to make the output neater.

0e7052e#r158890213

We should make efforts to avoid errors or warnings in the output of the notebooks, if this is possible!

0e7052e#r158890486

[tedinburgh]

Hi @Lima-PPGEP @LuizRaff @EstebanGarciaG this is my initial comments after the recent upload of the new versions of the notebooks.

General:

• It is very comprehensive, good job!
• I think some of the previous comments were not completely resolved yet (but please don't treat those comments as absolute, it is of course completely fine if you don't think they are all useful or valid!). But it would be helpful for me in reviewing if you can please respond any relevant comments directly (please note you can see the comment directly in place if you click on the link and then click on 'Load Diff' on the relevant file) or if you can make a note of which comments were addressed or not. To be honest this is partly a GitHub problem that I can't seem to directly compare diff of .ipynb files, so it took some time to identify what had changed.
• I think ideally the users should be able to actually run some code using the Jupyter notebook, and then potentially play around with different parameters etc. I don't think this is possible because the data for the examples appears to be stored locally and imported using a Google colab upload function. I am unsure what this should mean for the first dataset, which I believe is SINAN data that we can't share. I will suggest that we can use this data https://github.com/ISARICResearch/VERTEX/blob/main/projects/ARChetypeCRF_h5nx_synthetic_mf/vertex_patient_level_data/df_map.csv in place of the second example? I think the one you are using is showing errors and we need this to be working.
• Lots of great technical detail about assumptions, model checking etc, personally I think this section was structured well in each notebook. I ran out of time to go through all of the technical information in detail, but I read through a lot and I think the quality was mostly very high. However, I am not sure how technical these notebooks should be in view of the target audience, so just to repeat that I think feedback from a few potential users is more valuable than my review.
• Please would it be possible for comments from any other reviews to be on a GitHub issue (or PR) as well?
• I know you have already been doing this but I think it will be great if we can try to harmonise across the notebooks as much as possible.

Cox:

• I'm not familiar with COSSO (but if it is a good method to include that's fine!)
• I think residuals are not straightforward, but just to note that as far as I understand Martingale residuals are not centred on 0 and sometimes people use deviance residuals instead (which come from Martingale residuals).

GLM:

• Perhaps mention the assumptions about the distribution of epsilon?
• Maybe a bit more context about what residuals are and what is expected
• Could mention the implications of assumptions being violated (e.g. biased coefficients) and what to try when this happens

MICE:

• It's more code-heavy, which makes it a little bit harder to read

Logistic Regression

• No specific comments

[Lima-PPGEP]

Hi Tom, Thank you so much for your time and feedback, we really value that. We will do our best to make the improvements you pointed out. Sincerely, Carlos Lima UFPE-CTG/PPGEP Aluno Doutorado PPGEP | Pesquisa Operacional +55 81 99642 3176 Em sex., 11 de jul. de 2025 às 13:33, Tom Edinburgh < ***@***.***> escreveu:

…

*tedinburgh* left a comment (ISARICResearch/Training#4) <#4 (comment)> Hi @Lima-PPGEP <https://github.com/Lima-PPGEP> @LuizRaff <https://github.com/LuizRaff> @EstebanGarciaG <https://github.com/EstebanGarciaG> this is my initial comments after the recent upload of the new versions of the notebooks. General: - It is very comprehensive, good job! - I think some of the previous comments were not completely resolved yet (but please don't treat those comments as absolute, it is of course completely fine if you don't think they are all useful or valid!). But it would be helpful for me in reviewing if you can please respond any relevant comments directly (please note you can see the comment directly in place if you click on the link and then click on 'Load Diff' on the relevant file) or if you can make a note of which comments were addressed or not. To be honest this is partly a GitHub problem that I can't seem to directly compare diff of .ipynb files, so it took some time to identify what had changed. - I think ideally the users should be able to actually run some code using the Jupyter notebook, and then potentially play around with different parameters etc. I don't think this is possible because the data for the examples appears to be stored locally and imported using a Google colab upload function. I am unsure what this should mean for the first dataset, which I believe is SINAN data that we can't share. I will suggest that we can use this data https://github.com/ISARICResearch/VERTEX/blob/main/projects/ARChetypeCRF_h5nx_synthetic_mf/vertex_patient_level_data/df_map.csv in place of the second example? I think the one you are using is showing errors and we need this to be working. - Lots of great technical detail about assumptions, model checking etc, personally I think this section was structured well in each notebook. I ran out of time to go through all of the technical information in detail, but I read through a lot and I think the quality was mostly very high. However, I am not sure how technical these notebooks should be in view of the target audience, so just to repeat that I think feedback from a few potential users is more valuable than my review. - Please would it be possible for comments from any other reviews to be on a GitHub issue (or PR) as well? - I know you have already been doing this but I think it will be great if we can try to harmonise across the notebooks as much as possible. Cox: - I'm not familiar with COSSO (but if it is a good method to include that's fine!) - I think residuals are not straightforward, but just to note that as far as I understand Martingale residuals are not centred on 0 and sometimes people use deviance residuals instead (which come from Martingale residuals). GLM: - Perhaps mention the assumptions about the distribution of epsilon? - Maybe a bit more context about what residuals are and what is expected - Could mention the implications of assumptions being violated (e.g. biased coefficients) and what to try when this happens MICE: - It's more code-heavy, which makes it a little bit harder to read Logistic Regression - No specific comments — Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A7WOLVTCU6ZGT7SZ3CS7WJL3H7RNZAVCNFSM6AAAAAB6MZNLNKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTANRSHE2TIOJWGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>