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

JOSS Review: feedback on documentation #3

Closed
roualdes opened this issue Mar 22, 2024 · 3 comments
Closed

JOSS Review: feedback on documentation #3

roualdes opened this issue Mar 22, 2024 · 3 comments

Comments

@roualdes
Copy link

roualdes commented Mar 22, 2024

Overall the Kirstine.jl documentation is very nice. It walks users through a simple install process and a robust first example, the Sigmoid Emax model. Please find below some comments that I think could add some clarity to the package and its documentation, though admittedly I didn't make it through every page of the doc.

Relative to the Getting Started page

  • "fields fields" is probably a typo for just one "fields"
  • So the macros @simple_model and @simple_parameter appear to take as a first argument an arbitrary name and then append Model and Parameter, respectively, to the user specified first argument. Is this correct? This isn't explicitly mentioned, but it's my best understanding of what is going. Whether or not I'm right, it seems like there could be an extra sentence to help the user through this.
  • Can @simple_parameter reference only scalars? Does Kirstine.jl allow blocks within a parameter vector to be associated with one named parameter?

Relative to the Mathematical Background page

  • $H$is is supposed to be $H$ is, with a space between the Hessian and the word is. If a space is there already, I can't see it.
  • I found the two uses of $M$, namely $M$ and $M_T$, difficult to parse, especially once I got to the Gateaux Derivative section.
  • The JOSS draft paper and the doc Section Mathematical Background took reverse approaches to introduce complex equations. I tend to prefer the style in the JOSS paper, where sub-expressions are introduced on their own first, before using them within a more complex expression. For instance, in the doc Mathematical Background in subsection Objective Function, first the objective function is defined. The objective function relies on $M_T$, which relies on $M$, which relies on $F$. And only in the next sub-section is $\Psi$ introduced. This is all just an opinion here, so please don't change anything if you like it as is. If nothing else, please note that your documentation and your paper introduce the objective functions differently.

Relative to the home page of the doc

  • In the sub-section Rationale, you say "These are different tasks than what Kirstine.jl attempts to do." I think a sentence or two in the second paragraph about what explicitly separates the use-case of Kirstine.jl from the use cases of the R packages found on CRAN could be helpful.
@lsandig
Copy link
Owner

lsandig commented Mar 26, 2024

So the macros @simple_model and @simple_parameter appear to take as a first argument an arbitrary name and then append Model and Parameter, respectively, to the user specified first argument. Is this correct? This isn't explicitly mentioned, but it's my best understanding of what is going. Whether or not I'm right, it seems like there could be an extra sentence to help the user through this.

This behavior is documented in the docstrings of the two macros. I'll point this out more clearly when they are first used in the tutorial.

Can @simple_parameter reference only scalars? Does Kirstine.jl allow blocks within a parameter vector to be associated with one named parameter?

It only takes scalars, since I think that a (mathematical) vector of real numbers is the most common use case, and here the dimension of the parameter space is simply the number of scalar arguments. For anything more complicated (e.g. matrix or simplex parameters, parameter types with extra flags to indicate elements that should be considered known, ...) I think it's clearer to specify the type and dimension() method manually.

I found the two uses of M, namely M and MT, difficult to parse, especially once I got to the Gateaux Derivative section.

I'll try and see if I can find a better notation then; however, I've already exhausted most of the Latin and Greek alphabets. At least, using semantic math LaTeX macros for everything now pays off :)

The JOSS draft paper and the doc Section Mathematical Background took reverse approaches to introduce complex equations. […]

Thanks for pointing this out. Restructuring the math page from a large reference info dump to something more readable from top to bottom is already on my TODO list.

In the sub-section Rationale, you say "These are different tasks than what Kirstine.jl attempts to do." I think a sentence or two in the second paragraph about what explicitly separates the use-case of Kirstine.jl from the use cases of the R packages found on CRAN could be helpful.

Ok, I'll make it clearer that these design packages are for ANOVA/linear/polynomial regression models, while Kirstine.jl deals with nonlinear models.

@lsandig
Copy link
Owner

lsandig commented Mar 27, 2024

I've worked on the documentation for a bit, and incorporated your suggestions. The changes are on the development branch over at sourcehut, starting at 00aef6b. I don't have rendered html documentation for the development branch set up, I hope this is not too inconvenient for you to read.

@roualdes
Copy link
Author

Great to see that you used some of my suggestions to update the doc. I'm happy for you to close the issue at your convenience.

@lsandig lsandig closed this as completed Apr 2, 2024
@harisorgn harisorgn mentioned this issue Apr 3, 2024
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

2 participants