search

metrumresearchgroup / Torsten

library of C++ functions that support applications of Stan in Pharmacometrics
BSD 3-Clause "New" or "Revised" License
52 stars 11 forks source link

Manual: correct function signatures #6

Closed charlesm93 closed 3 years ago

charlesm93 commented 5 years ago

Main issues

The signature of Torsten functions is inaccurate. Torsten functions return a matrix, not a 2-d or 1-d array, as indicated in the user manual.

Also the user manual under the example-models directory is different from the user manual in the doc directory. I propose to remove the former, and only keep the latter.

Other issues

I'll also proofread the manual, correct minor typos, and add certain missing references.

Another issue is the examples: they do not take advantage of new syntax features which would make the code significantly shorter. Based on @yizhang-cae 's comment in issue #4 this problem can be addressed by directly modifying the examples. I'm happy to do it, using the code I developed for the Torsten workshop at U. Buffalo. That said, I'd rather avoid interfering with some of the restructuring @billgillespie may be working on.

Finally, there is the technical appendix (for a draft, see https://github.com/charlesm93/presentations/blob/master/TorstenAppendix/Torsten_appendix.pdf), but this is less an issue than a milestone for releasing Torsten v1.0.

How to verify

Whoever reviews the pull request will need to:

charlesm93 commented 5 years ago

@yizhang-cae Can you give me some details on how to recompile the user manual? Plain Tex shop doesn't work, and the error message recommends invoking latex with the shell-escape flag. I'm assuming texing must be done from the command line?

yizhang-yiz commented 5 years ago

I use minted + pygments for code highlighting. So you’ll need them, as well as lexer for stan at https://github.com/jrnold/pygments_bugs

Minted is a latex package and should be shipped with a standard installation. Pygments is a python library, http://pygments.org/ you’ll find instructions there easy to follow.

-YZ

On Mar 24, 2019 at 8:06 AM, <Charles Margossian (mailto:notifications@github.com)> wrote:

@yizhang-cae (https://github.com/yizhang-cae) Can you give me some details on how to recompile the user manual? Plain Tex shop doesn't work, and the error message recommends invoking latex with the shell-escape flag. I'm assuming texing must be done from the command line?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/metrumresearchgroup/Torsten/issues/6#issuecomment-475967764), or mute the thread (https://github.com/notifications/unsubscribe-auth/AfkhsGZ5sY93GdBFECm4wY3kJpb2Qmkjks5vZ5RqgaJpZM4cFiq-).

charlesm93 commented 5 years ago

Looks like I have pygments from Anaconda; and I updated minted from version 2.0 to 2.5. I changed the engine command in the preferences of TeXShop to pdflatex --file-line-error --synctex=1 --shell-escape. My understanding is that --shell-escape is required to link minted to pygmentize, but I still get an error message stating: You must have pygmentize installed to use this package.

Based on this thread, https://github.com/SublimeText/LaTeXTools/issues/657, I need to change the tex path, but I have not found a way of doing this, either with TexShop or from the command line. Honestly, I'm a bit numbed by how much time I've spent on this, but I'll give it another go in the next days.

EDIT: this page prescribes of way of checking the path where pigmentize is located: https://tex.stackexchange.com/questions/48018/minted-not-working-on-mac

charlesm93 commented 5 years ago

Ok, I got it to work. It seems there are some issues with TexShop. I'm assuming you use a different editor? From the command line, it works to type pdflatex --file-line-error --shell-escape --synctex=1 torsten_manual.tex

yizhang-yiz commented 5 years ago

Great. The manual generation was tested in shell with texlive backend on Linux and Mac.

On Mar 25, 2019 at 6:51 AM, <Charles Margossian (mailto:notifications@github.com)> wrote:

Ok, I got it to work. It seems there are some issues with TexShop. I'm assuming you use a different editor? From the command line, it works to type pdflatex --file-line-error --shell-escape --synctex=1 torsten_manual.tex

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/metrumresearchgroup/Torsten/issues/6#issuecomment-476205805), or mute the thread (https://github.com/notifications/unsubscribe-auth/AfkhsOwtt6N79fwWFELF72ZgPpnrNpXQks5vaNRJgaJpZM4cFiq-).

yizhang-yiz commented 5 years ago

I like the technical appendix. Maybe we can expand it into something akin to theory guide that some sientific software use to complements the user manual. For that we may want to focus on torsten numerical models and just point to stan for autodiff . In particular, we need to add how the steady state is treated.

-YZ

On Mar 25, 2019 at 6:51 AM, <Charles Margossian (mailto:notifications@github.com)> wrote:

Ok, I got it to work. It seems there are some issues with TexShop. I'm assuming you use a different editor? From the command line, it works to type pdflatex --file-line-error --shell-escape --synctex=1 torsten_manual.tex

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/metrumresearchgroup/Torsten/issues/6#issuecomment-476205805), or mute the thread (https://github.com/notifications/unsubscribe-auth/AfkhsOwtt6N79fwWFELF72ZgPpnrNpXQks5vaNRJgaJpZM4cFiq-).

charlesm93 commented 5 years ago

I made changes to the manual. If I'm given the credentials, I'll create a new branch and submit a pull request. A summary of the changes:

It seems section 1.3, Development Plans may need some updates. @yizhang-cae you'll know better than me how to handle this. The examples still need to be updated. I didn't see in the tex code how these were linked to the code under example-models.

I'm happy to discuss the scope of the technical appendix. But yeah, a theory guide sounds good.

yizhang-yiz commented 5 years ago

I've sent invite. Please note that Torsten repo is just a container. Anything other than documentation should be updated in its original repo, that includes:

https://github.com/metrumresearchgroup/math https://github.com/metrumresearchgroup/stan https://github.com/metrumresearchgroup/cmdstan https://github.com/metrumresearchgroup/example-models

I suggest we keep the old doc in example-models repo before finalizing the new version.

charlesm93 commented 5 years ago

Sounds good. I'll update the examples in example-models. We can then propagate the code to what's in the user manual.

Minor issue: I'm not able to change the date that appears on the manual. It says October 2018 but it should be march 2019.

charlesm93 commented 5 years ago

If you don't mind the wrong date, or want to fix it later, I can submit the PR. I'll then update the example code.

yizhang-yiz commented 5 years ago

Don’t worry about the date.

-YZ

On Apr 1, 2019 at 12:51 PM, <Charles Margossian (mailto:notifications@github.com)> wrote:

If you don't mind the wrong date, or want to fix it later, I can submit the PR. I'll then update the example code.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (https://github.com/metrumresearchgroup/Torsten/issues/6#issuecomment-478721200), or mute the thread (https://github.com/notifications/unsubscribe-auth/AfkhsDTlZAkHTgCwz14yXwfSo6VXJ_xnks5vcmM_gaJpZM4cFiq-).

charlesm93 commented 5 years ago

I'm going to transpose the changes to the new manual and also proofread it. The goal is to have a polished version before Stan Con.