openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
694 stars 36 forks source link

[REVIEW]: VineCopulas: an open-source Python package for vine copula modelling #6728

Open editorialbot opened 2 months ago

editorialbot commented 2 months ago

Submitting author: !--author-handle-->@judithclaassen<!--end-author-handle-- (Judith Claassen) Repository: https://github.com/VU-IVM/VineCopulas Branch with paper.md (empty if default branch): Version: v0.2.5 Editor: !--editor-->@ymzayek<!--end-editor-- Reviewers: @haozcbnu, @couasnonanais Archive: Pending

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/5118d62bcc8a3621ebdd2955f8e88405"><img src="https://joss.theoj.org/papers/5118d62bcc8a3621ebdd2955f8e88405/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/5118d62bcc8a3621ebdd2955f8e88405/status.svg)](https://joss.theoj.org/papers/5118d62bcc8a3621ebdd2955f8e88405)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@haozcbnu & @couasnonanais, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @ymzayek know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Checklists

📝 Checklist for @haozcbnu

📝 Checklist for @couasnonanais

editorialbot commented 2 months ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 2 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.07 s (336.7 files/s, 144225.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                           7            718            556           5284
CSV                              2              0              0           1408
Jupyter Notebook                 2              0           1742            264
Markdown                         5            103              0            262
TeX                              1             17              0            127
YAML                             6             18             20            119
TOML                             1              5              0             37
reStructuredText                 1             11              7             11
-------------------------------------------------------------------------------
SUM:                            25            872           2325           7512
-------------------------------------------------------------------------------

Commit count by author:

   109  Claassen
    52  Judith Claassen
     9  Jäger
     6  Elco Koks
     1  Koks
editorialbot commented 2 months ago

Paper file info:

📄 Wordcount for paper.md is 1016

✅ The paper includes a Statement of need section

editorialbot commented 2 months ago

License info:

🟡 License found: GNU General Public License v3.0 (Check here for OSI approval)

editorialbot commented 2 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.3389/fnins.2022.910122 is OK
- 10.1201/b17116 is OK
- 10.1146/annurev-statistics-040220-101153 is OK
- 10.5281/ZENODO.10435751 is OK
- 10.1214/aos/1031689016 is OK
- 10.1201/9780367803896 is OK
- 10.5194/nhess-23-2251-2023 is OK
- 10.5194/hess-21-2701-2017 is OK

MISSING DOIs

- No DOI given, and none found for title: copula: Multivariate Dependence with Copulas
- No DOI given, and none found for title: VineCopula: Statistical Inference of Vine Copulas
- No DOI given, and none found for title: CDVineCopulaConditional: Sampling from Conditional...
- No DOI given, and none found for title: copulas: Create tabular synthetic data using copul...
- No DOI given, and none found for title: Monte Carlo simulation of vine dependent random va...

INVALID DOIs

- None
ymzayek commented 2 months ago

@haozcbnu and @couasnonanais

Dear reviewers, you can start your review by creating your tasklist with the following command:

@editorialbot generate my checklist

In that list, there are several tasks. Whenever you perform a task, you can check on the corresponding checkbox. You can also reference the JOSS reviewer guidelines which is linked in first comment in this thread. Since the review process of JOSS is interactive, you can always interact with the author, the other reviewers, and the editor during the process. You can open issues and pull requests in the target repo. Please mention the url of this page in there so that we can keep tracking what is going on.

Thank you in advance.

editorialbot commented 2 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

haozcbnu commented 1 month ago

Review checklist for @haozcbnu

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

ymzayek commented 1 month ago

@couasnonanais let me know if you are able to get started on this soon. Thanks!

couasnonanais commented 1 month ago

Review checklist for @couasnonanais

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

editorialbot commented 1 month ago

Hello @couasnonanais, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for branch
@editorialbot set joss-paper as branch

# Run checks and provide information on the repository and the paper file
@editorialbot check repository

# Check the references of the paper for missing DOIs
@editorialbot check references

# Generates the pdf paper
@editorialbot generate pdf

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Get a link to the complete list of reviewers
@editorialbot list reviewers
haozcbnu commented 1 month ago

Here are several comments/suggestions.

Lines 9-10: Here some references can be given to introduce the basic concept of Copula.

Lines 15-16: please check the punctuation mark.

Lines 19-22: Here the concept of the "curse of dimensionality " can be introduced to demonstrate the challenge in multivariate dependence modeling.

Captions of Figure 1: D) What do you mean by " As not every vine copula structure is suitable to generate conditional samples of every variable"? E) Drawing conditional samples of the variable 1 given specific values of variables 2 and 3? This needs to be clear.

Lines 59-61: To emphasize the usefulness of this package, the authors can highlight the large number of variables or hazards (>2) in the multi-hazard phenomenon.

ymzayek commented 1 month ago

Hello @couasnonanais do you have an update on your review?

ymzayek commented 1 month ago

Here are several comments/suggestions.

Lines 9-10: Here some references can be given to introduce the basic concept of Copula.

Lines 15-16: please check the punctuation mark.

Lines 19-22: Here the concept of the "curse of dimensionality " can be introduced to demonstrate the challenge in multivariate dependence modeling.

Captions of Figure 1: D) What do you mean by " As not every vine copula structure is suitable to generate conditional samples of every variable"? E) Drawing conditional samples of the variable 1 given specific values of variables 2 and 3? This needs to be clear.

Lines 59-61: To emphasize the usefulness of this package, the authors can highlight the large number of variables or hazards (>2) in the multi-hazard phenomenon.

@haozcbnu can you clarify the status regarding these comments? Any updates there?

judithclaassen commented 1 month ago

Here are several comments/suggestions. Lines 9-10: Here some references can be given to introduce the basic concept of Copula. Lines 15-16: please check the punctuation mark. Lines 19-22: Here the concept of the "curse of dimensionality " can be introduced to demonstrate the challenge in multivariate dependence modeling. Captions of Figure 1: D) What do you mean by " As not every vine copula structure is suitable to generate conditional samples of every variable"? E) Drawing conditional samples of the variable 1 given specific values of variables 2 and 3? This needs to be clear. Lines 59-61: To emphasize the usefulness of this package, the authors can highlight the large number of variables or hazards (>2) in the multi-hazard phenomenon.

@haozcbnu can you clarify the status regarding these comments? Any updates there?

@haozcbnu Thank you so much for providing us with your review. @ymzayek I would prefer to incorporate the suggestions after having received @couasnonanais review as well, if possible.

couasnonanais commented 1 month ago

Hi!

I will send my review on Friday. Thanks,

Anaïs

Le mar. 28 mai 2024 à 20:44, Yasmin @.***> a écrit :

Hello @couasnonanais https://github.com/couasnonanais do you have an update on your review?

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/6728#issuecomment-2135896169, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG2TOANYL3BUKA5X2LGSTZLZETGBVAVCNFSM6AAAAABHKVOUQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZVHA4TMMJWHE . You are receiving this because you were mentioned.Message ID: @.***>

ymzayek commented 3 weeks ago

@couasnonanais let me know if you will have time to finish your review soon. If for whatever reason it is not possible for you that is ok but then it might be necessary to look for a new reviewer. Let me know if I can be of any help. Thanks!

couasnonanais commented 3 weeks ago

Hi Yasmin,

Apologies for the delay. I have started the review and will upload it tomorrow.

Anaïs

On Tue, 11 Jun 2024, 13:57 Yasmin, @.***> wrote:

@couasnonanais https://github.com/couasnonanais let me know if you will have time to finish your review soon. If for whatever reason it is not possible for you that is ok but then it might be necessary to look for a new reviewer. Let me know if I can be of any help. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/6728#issuecomment-2160577705, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG2TOAMTDHWY5MLEQQFOMFDZG3Q3ZAVCNFSM6AAAAABHKVOUQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRQGU3TONZQGU . You are receiving this because you were mentioned.Message ID: @.***>

couasnonanais commented 1 week ago

Hi @ymzayek, @haozcbnu and @judithclaassen,

Apologies for the delay in my review. I very much enjoyed having a look at the package and I think it is a great contribution to the Python community.

I have added my comments as issues (https://github.com/VU-IVM/VineCopulas/issues/16#issue-2373323282 and https://github.com/VU-IVM/VineCopulas/issues/15#issue-2372600177).

Thank you,

Anaïs

ymzayek commented 1 week ago

Thank you @couasnonanais !

ymzayek commented 6 days ago

Hi just checking in to see if there are any updates here. @judithclaassen do you have everything you need to continue addressing the reviews?

judithclaassen commented 14 hours ago

@ymzayek Thank you for checking on the review process. @couasnonanais @haozcbnu thank you both so much for taking the time to review. Your comments have helped us to improve the manuscript and documentation. I would like to highlight how the comments have been implemented.

Response to @haozcbnu :

Lines 9-10: Here some references can be given to introduce the basic concept of Copula.

Reference added in L9-10 and L15

Lines 15-16: please check the punctuation mark.

corrected

Lines 19-22: Here the concept of the "curse of dimensionality " can be introduced to demonstrate the challenge in multivariate dependence modeling.

While this can be relevant to, for example defining the CDF of a vine copula, we think it is slightly to specific for a JOSS paper, which is focused more on the functionality and need for a broader audience. We do have some information about the increasing complexity of higher dimensions in the documentation.

Captions of Figure 1: D) What do you mean by " As not every vine copula structure is suitable to generate conditional samples of every variable"? E) Drawing conditional samples of the variable 1 given specific values of variables 2 and 3? This needs to be clear.

Added clarification in the caption

Lines 59-61: To emphasize the usefulness of this package, the authors can highlight the large number of variables or hazards (>2) in the multi-hazard phenomenon.

added an additional variable in L 68-69

Response to @couasnonanais

Paper:

It is not obvious at first that the letters in the caption refer to the arrows.

Clarified in the caption

In the Table with the Data, I would suggest to change the notation to be in line with common mathematical notations. Therefore, I would add next to Variable 1 (𝑋1), etc. In the table, the notation then becomes 𝑥11 until 𝑥1𝑛 and for 𝑋2: 𝑥21 until 𝑥2𝑛. Calling this table as Table 1 data or somehting like that can then clarify the caption (see next point)

Table has been adjusted

In the caption after (A), you could then mention that "Samples from Table 1 - data, consisting of both continuous and discrete varibles (plotted in blue) are transformed into pseudo-observations using their marginal distributions (shown in green)". It would be good to mention pseudo-observations/pseudo-data there too (I provide a suggestion but feel free to use it or not)

Suggestion implemented

It is not clear who the target audience is. I understand after reading the text that one type of audience is Python users and from the examples, someone who has some mathematical background. You could add a few sentences about this in the paper to explicitely adress this (since this is in the review checklist)

Addressed in L63-65

I understand that the paragraph between line 49 and 53 lists the unique features of the package. I would make this more explicit. (for e.g. "The unique features of this package is that is integrates....") or even using a bullet point list. Here I would clarify "fitting CDF and PDF" given the question you received as an issue (issue PDF and CDF calculation! #14 )

Adjusted L 50-52 to incorporate this. Also added a Density function to the package and will address the issue.

You could then extend this bullet list to the main functionalities of the package (as highlighted in the examples)

As there is a bullet point list on the documentation home page, github and PyPI homepage, we have decided not to implement a bullet point list in the paper but keep the functionality in a paragraph instead.

I would add all references that are mentioned in the example Python notebooks as I would argue that they are key references.

Implemented across the paper

In the Statement of need, I would highlight the paragraph

Unsure which paragraph to highlight. Please clarify if relevant

Typos etc: • line 50: CDF --> you mean cumulative here, correct? • line 51: random sample generation. (sampling --> sample) • line 58: Python • line 54: reference missing for the copulas package? • The last sentences reads a bit strange because these studies have used alternative coding languages or Python package. Maybe rephrase to put the emphasis on the need for more complete package to apply for these high dimensionality and complex problem. But that also can welcome contributions from the growing Python community

All typos have been addressed

it would be good to mention or point the user towards the list of univariate distributions available for the fit of the marginal distributions. And explicitely mention the scipy package.

Added in L52-53

Adding the Getting Started section to the home page of the Documentation

Added section to the home page of the documentation

Maybe in general expand the introduction to the homepage.

Addressed by implementing the previous comment and the following comment that already make the Homepage of the documentation more complete.

Point towards examples after each bullet point of what the package can do

in the bullet point list on the homepage we hade added hyperlinks to the relevant information in the documentation to guide the reader.

judithclaassen commented 14 hours ago

@editorialbot generate pdf

editorialbot commented 14 hours ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left: