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

flepimop.org documentation updates for information pertaining to new users #460

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

emprzy
Copy link
Collaborator

@emprzy emprzy commented Jan 9, 2025

Describe your changes.

This pull request...

  • Changes documentation to reflect imminent changes in user local installation processes (affecting 'before any run' and 'quick start guide' sections of the flepimop.org documentation),
  • Fully eradicates flepimop_sample from code and documentation; replacing it with the flepiMoP/examples/tutorials directory,
  • Replaces all environmental variable instances of DATA_PATH with PROJECT_PATH. It does not attempt to adjust the command line option --data_path, as I believe @TimothyWillard said he was working on editing the CLI options.

Does this pull request make any user interface changes? If so please describe.

New users will be able to install flepiMoP (all packages and dependencies along with initialization of a conda environment) with a single script that they can run from CL.

What does your pull request address? Tag relevant issues.

This pull request addresses necessary documentation changes resulting from issue GH #401 and pull request GH #418.
Because of eradication of flepimop_sample, it also serves GH #368.

Changing documentation to reflect upcoming changes in user local installation processes and moving towards eliminating `flepimop_sample` as a step in running for new users.
@TimothyWillard TimothyWillard added documentation Relating to ReadMEs / gitbook / vignettes / etc. medium priority Medium priority. labels Jan 10, 2025
@emprzy emprzy marked this pull request as ready for review January 10, 2025 20:09
@emprzy
Copy link
Collaborator Author

emprzy commented Jan 10, 2025

My question to reviewers is: should I go ahead and search for all references to flepimop_sample in other places in the documentation and replace them with flepiMoP/examples/tutorials, or should I do that in another PR?

@TimothyWillard
Copy link
Contributor

My question to reviewers is: should I go ahead and search for all references to flepimop_sample in other places in the documentation and replace them with flepiMoP/examples/tutorials, or should I do that in another PR?

My vote would be to do it all in one pass as to minimize confusion.

@emprzy
Copy link
Collaborator Author

emprzy commented Jan 17, 2025

places where flepimop_sample is referenced (need to be resolved):

HOW TO RUN

  • before any run
  • quick start guide
  • advanced run guides ->
  • running with a docker locally
  • running locally in a conda environment
  • running on a HPC with slurm

GEMPYOR: MODELING INFECTIOUS DISEASE DYNAMICS

  • model output

MODEL INFERENCE

  • inference model output

@TimothyWillard
Copy link
Contributor

places where flepimop_sample is referenced (need to be resolved):

I'm going to hold off on reviewing until this, unless you want me to review now @emprzy?

@emprzy
Copy link
Collaborator Author

emprzy commented Jan 27, 2025

A question for @pearsonca (or anyone else that cares to chime in). We are very close to purging data_path as a config key from the flepiMoP codebase, but DATA_PATH as an environmental variable referring to a place where data exists is still present in the documentation and code. How would we feel about universally moving towards using PROJECT_PATH to refer to these instances (unless there is a distinction between PROJECT_PATH and DATA_PATH that I am not familiar with)? I've included an example each of where PROJECT_PATH exists in the documentation, and where DATA_PATH does, and you can let me know what you think:

Screenshot 2025-01-27 at 1 39 50 PM from how-to-run quick start guide Screenshot 2025-01-27 at 1 44 47 PM from how-to-run useful commands

This does not change `--data_path`, the CL option, just the envvar `DATA_PATH`.
@emprzy
Copy link
Collaborator Author

emprzy commented Jan 31, 2025

Sorry for the mid-way expansion of what this PR covers, but it's ready for review now!

@TimothyWillard TimothyWillard added the next release Marks a PR as a target to include in the next release. label Feb 3, 2025
Copy link
Contributor

@pearsonca pearsonca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

progress, but seems like several open items.


* the directory containing the flepimop code (likely the folder you cloned from Github), which we'll call `FLEPI_PATH`
* the directory containing your project code including input configuration file and population structure (again likely from Github), which we'll call `DATA_PATH`
Take note of the location of the directory on your local computer where you cloned the flepiMoP model code (which we'll call `FLEPI_PATH`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it too much to also explain that it is literally defined as an environmental variable with the same name? we don't want to overwhelm folks, but that also seems like a useful note.

Comment on lines +80 to 85
If you're on a **Mac** or Linux/Unix based operating system, define the FLEPI\_PATH and PROJECT\_PATH environmental variables to be your directory locations, for example

```bash
export FLEPI_PATH=$(pwd)/flepiMoP
export DATA_PATH=$(pwd)/flepimop_sample
export FLEPI_PATH=/Users/YourName/Github/flepiMoP
export PROJECT_PATH=/Users/YourName/Github/flepiMoP/examples/tutorials
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't want people to do some of this, right? as in, installing flepimop should set the flepipath?

<summary>Help! I have errors in installation</summary>

If you get an error because no cran mirror is selected, just create in your home directory a `.Rprofile` file:
You can check that the variables have been set by either typing `env` to see all defined environmental variables, or typing `echo $FLEPI_PATH` to see the value of `FLEPI_PATH`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably shouldn't suggest people env - going to spam them confusingly.

Comment on lines +98 to +100
<pre class="language-bash"><code class="lang-bash"><strong>set FLEPI_PATH=C:\Users\YourName\Github\flepiMoP
</strong>set PROJECT_PATH=C:\Users\YourName\Github\flepiMoP\examples\tutorials
</code></pre>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, what part of this is now in the install action, such that we don't want them to do it?


Other environmental variables can be set at any point in process of setting up your model run. These options are listed in ... ADD ENVAR PAGE
Other environmental variables can be set at any point in process of setting up your model run. These options are listed in ... **ADD ENVAR PAGE**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind a live TODO, but it should be in a comment, rather than actively displayed. Probably a better version is to convert this to an item on an explicit issue, e.g. the new environmental variables one. Basically an explicit note "in file XYZ, section ABC, ..."


For example, some frequently used environmental variables which we recommend setting are:
For example, some frequently used environmental variables we recommend setting are:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we recommend setting these? i don't think this is true

rm -r model_output/ # delete the outputs of past run if there are
```

#### Inference run

An inference run requires a configuration file that has an `inference` section. Stay in the `$DATA_PATH` folder, and run the inference script, providing the name of the configuration file you want to run (ex. `config.yml`). In the example data folder (flepimop\_sample), try out the example config XXX.
An inference run requires a configuration file that has an `inference` section. Stay in the `$PROJECT_PATH` folder, and run the inference script, providing the name of the configuration file you want to run (ex. `config.yml`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the inference action, right? it's a script under the hood, but not from the user perspective

@@ -82,7 +81,7 @@ If you'd like to have more control, you can specify the arguments manually:
$ python $FLEPI_PATH/batch/inference_job_launcher.py --slurm \
-c $CONFIG_PATH \
-p $FLEPI_PATH \
--data-path $DATA_PATH \
--data-path $PROJECT_PATH \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't be --data-path any more, right?

@@ -11,7 +11,7 @@ description: >-
See the [Before any run](../before-any-run.md) section to ensure you have access to the correct files needed to run. On your local machine, determine the file paths to:

* the directory containing the flepimop code (likely the folder you cloned from Github), which we'll call `<dir1>`
* the directory containing your project code including input configuration file and population structure (again likely from Github), which we'll call `<dir2>`
* the directory containing your project code including input configuration file and population structure, which we'll call `<dir2>`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...weird. why isn't this environmental variables again? docker supports those. maybe not something to fix here, but if not, should make it into an issue.


This repository mirrors the contents in the **examples/tutorial** folder of the FlepiMoP repository ([link](https://github.com/HopkinsIDD/flepiMoP/tree/main/examples/tutorials)). It can be used to try out running simple projects using `flepimop` code and as a template for new projects.
This directory can be used to try out running simple projects using `flepimop` code and as a template for new projects. It mirrors now deprecated example repository `flepimop_sample`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This directory can be used to try out running simple projects using `flepimop` code and as a template for new projects. It mirrors now deprecated example repository `flepimop_sample`.
This directory can be used to try out running simple projects using `flepimop` code and as a template for new projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Relating to ReadMEs / gitbook / vignettes / etc. medium priority Medium priority. next release Marks a PR as a target to include in the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants