Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reviews CMSB #79

Open
13 of 41 tasks
xhajnal opened this issue Jul 2, 2020 · 1 comment
Open
13 of 41 tasks

reviews CMSB #79

xhajnal opened this issue Jul 2, 2020 · 1 comment

Comments

@xhajnal
Copy link
Owner

xhajnal commented Jul 2, 2020

RV 1:

  • the formal notation is a bit sloppy;
    \ can you be more specific?

  • figures seem low resolution and too small to be fully read.
    \ I have no problem in the pdf version, and is 800x550 low resolution?

  • Please improve notation at the end of page 2, i.e.:

    • - V = { x_1, \ldots, x_n }
    • - for consistency give a name to the sets in the third and fourth item?
    • - You may want to denote the last element by the dector d = (d_1, \ldots, d_m), d \in \mathbb{R}^m
    • Notation should also be improved for the first two items on page 3.
      \ Notation Updated but can you be more specific?
  • It is not clear how the empirical estimates of satisfaction probabilities are provided:are those averages across repeated experiments each giving a Bernoulli sample about the property being satisfied at each observation?
    \ In this work, we consider data as a point estimate of satisfaction. Nothing more, nothing less.
    In the case study

  • Figure 1 is never mentioned in the runnning text?

  • Figure 1: I would also consider refactoring it: the textual content is hardly readable due to poor resolution and tiny font sizes.
    \ How is that I can read it without any problems in the pdf?

  • There is some componentry illustrated in Fig. 2 which is not described in the place where the figure is mentioned (e.g., mpmath?).
    \ No, there is none

  • The tool's web page is mentioned twice, in Section 1 and in Section 3.
    \ So what?

  • Section 3 dives into space sampling directly; in my opinion there should be a paragraph about the tool implementation more in general.
    \ gimme more space and I put the GUI description in respective part back in

  • I did not understand the sentence at the end of Section 3.1.
    \ we can put an example in the tutorial but finishing the manual would be the best.

  • In Section 3.2, I do not understand what "level-order" means.
    \ that is probably because we did not define it, but googling could help
    \ I mean, should I put a link to Wikipedia or what?

  • There are only two references to class-level implementation details in Section 3. Consider removing them to keep with the higher level of the rest of the paper.

  • Font sizes in Fig. 3 are too small.

  • Improve the formatting of Table 1 (number of digits, scientific notation, etc.)
    \ like what?

  • Section 5: too many short paragraphs?
    \ Should I make it as one paragraph or itemize?

Minor comments:

  • - "formatdeclares"
  • - "data encode": not clear what it means
  • - "plain divided" -> "plane divided"

RV 2:

  • However, more detail on the workflow, including an example, would guarantee easier access to the content.
    \ more detail in HSB paper and the example in the tutorial (Which is written there in the text)
  • more details on the next steps of the project would be desirable.
    \ I can give a link on the git with future plans
  • Missing space on page 3: The model file in .pm formatdeclares a discrete time Markov chains with its parameters.
  • Be instead of he on page 4: Source code of the tool with previously published Jupyter notebooks can he find on GitHub - https://github.com/xhajnal/DiPS.

RV 3:

  • I was wondering how easy it would be to adapt the approach for models with a CTMC semantics?
    \ me too :D
  • A comparison with the tool "Mitra ED, Suderman R, Colvin J, Ionkov A, Hu A, Sauro HM, Posner RG, Hlavacek WS. PyBioNetFit and the biological property specification language. IScience. 2019 Sep 27;19:1012-36." might be interesting, as it also addresses the problem of scarcely available data by allowing to constrain the parameter space by multiple qualitative property specifications.
    \ They solve simulation-based optimisation of BNGL/SMBL model to quantitative/qualitative data (partial time series/ constraints of concentration eg. [X] > [Y] @ t=10s) using gradient-less methods
    \ Qualitative is done with their specific logic, ~ LTL with at operator plus weights on the constraints, computing min/max of time period of focus
  • I missed a concrete example in the tutorial and the paper. The current example is abstract. Therefore, it is difficult to assess how easy it is to find properties and define them in PCTL to constrain the parameter space, to identify situations where it makes sense to apply the proposed method, or how the proposed approach of parameter synthesis compares to others.
    \ concrete example added

RV 4 (Tool eval):

  • I found the tool, in particular its presentation at github, very immature.
    \ can you be more specific?
  • The tutorial (and the github) includes only one and very trivial model (9 states) -- the induced synthesis problem is very simple (i.e. leads to simple polynomial functions).
    \ that is EXACTLY why it is in the tutorial, would you like to rather read 30MB of text and wonder how did you get these?
  • I miss an interpretation of the obtained results -- the tutorial should clearly explain how to interpret the results.
    \ partly added
  • a reasonable set of examples allowing to judge the applicability and usability of the tool.
  • There is no sign of a systematic testing of the tool.
    \ who is asking for it? and btw, check the code
  • I found some results surprising (e.g. the probability intervals for the example is smaller than 0 and higher than 1)
    \ I would find the confidence intervals of probabilities to be below zero or greater than 1 strange/surprising, but it is what we say in the paper and documentation
    • speaking of which we should update it,
  • some behaviour of the tool very
    • strange (e.g. modifying the data via GUI caused an error and file corruption).
      • not replicable
  • There is no scalability analysis — it is impossible to predict the limitation of the tool (number of states, parameters, data points).
    \ there is for refinement, have a look on HSB paper
  • The manual is incomplete: Chapter 3 includes only section titles.
    \ manual is under construction, it was not necessary to have it, we can delete it until it is done
  • The parameters and settings of the underlying methods (available in GUI) are not described.
    \ they are described - roll over the parameter, moreover see the documentation in the code

I am very pleased to receive any additional feedback from Verena on the tool, although for me it seems a bit weird to be punished for something which was not directly mentioned to be evaluated by the Tool Eval members:
- scalability
- systematic testing
- manual (although I am not sure if we speak about the same thing as we do not have the manual so far)

@xhajnal
Copy link
Owner Author

xhajnal commented Jul 2, 2020

Moreover:
I see 3 grades: -1, -1/0, and -1 where I was expecting the last (overall mark) be the score, which is -3

@xhajnal xhajnal changed the title reviews reviews CMSB Jul 29, 2020
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

No branches or pull requests

1 participant