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

Add Beginner Tutorials to the GETTSIM Documentation. #232

Merged
merged 48 commits into from Oct 21, 2020
Merged

Conversation

amageh
Copy link
Member

@amageh amageh commented Aug 12, 2020

What problem do you want to solve?

We (David, Tim, & Annica) will use this PR to work on a set of four tutorials for the documentation that are targeted mainly towards beginner's first contact with GETTSIM. The tutorials build on and expand the existing interface tutorial. The following four tutorials are planned:

  1. Basic Interface: Will be very similar to the existing interface tutorial but includes some adjustments to align it coherently with the following tutorials. Includes a minimal example of a possible input and use case.
  2. Advanced Interface: Introduces larger simulated datasets to showcase the different combinations of input and target variables that GETTSIM supports.
  3. Parameters: Showcases how parameters in params_dict can be changed to make changes to the policy environment.
  4. Policy Functions: Showcases how policy functions can be overridden to make changes to the policy environment.

Feedback is very welcome!

Todo

  • Target the right branch and pick an appropriate title.
  • Tutorial Basic Interface
  • Tutorial Advanced Interface
  • Tutorial Parameters
  • Tutorial Policy Functions
  • If it is a relevant contribution, put it in CHANGES.rst.
  • Any steps that still need to be done.

@amageh amageh added the documentation Improvements or additions to documentation label Aug 12, 2020
@codecov
Copy link

codecov bot commented Aug 12, 2020

Codecov Report

Merging #232 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #232   +/-   ##
=======================================
  Coverage   91.38%   91.38%           
=======================================
  Files          60       60           
  Lines        2101     2101           
=======================================
  Hits         1920     1920           
  Misses        181      181           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db3d039...793d8bd. Read the comment docs.

@amageh amageh self-assigned this Aug 12, 2020
@amageh
Copy link
Member Author

amageh commented Sep 28, 2020

These are our drafts for the four tutorials listed in the PR description. The documentation build for this PR can be found here. We have been working on the status of the master branch version of GETTSIM, i.e. the conda installation is behind.

Some questions that are still open:

  • We generally had some troubles in coming up with suitable datasets and ended up using some from the lectures. We are happy for other suggestions. I think in our meetup a while ago, there was some dicussion about creating a simulated dataset (or function that simulates a dataset) for various purposes. Such a function would probably be quite useful.
  • We are not very confident in the way inputs and outputs are explained. In the tutorials, we refer to the crosswalk page in the documentation but this seems to not be completely up to date? (i.e. there is a note in the basic tutorial on this list being somewhat wrong, that still needs to be removed)
  • In the policy_functions tutorial we 'overwrite' arbeitsl_geld_2_m_hh but this is actually not a key in the policy_functions dictionary before. The implementation seems to work anyway but we are not quite sure why (and think we should probably specify this more clearly).

We are happy to receive comments and feedback!

@amageh
Copy link
Member Author

amageh commented Oct 6, 2020

Great! Thank you very much! I agree that the existing interface tutorial can be removed. What do you mean by basic tutorial?

I just meant the basic interface tutorial we added. It ended up being a very similar (just slightly more basic) version of the existing interface tutorial.

@ChristianZimpelmann
Copy link
Member

Great! Thank you very much! I agree that the existing interface tutorial can be removed. What do you mean by basic tutorial?

I just meant the basic interface tutorial we added. It ended up being a very similar (just slightly more basic) version of the existing interface tutorial.

Ok, I see! I understood that there are two duplicates.

From my side, it can be merged now.

@hmgaudecker
Copy link
Collaborator

hmgaudecker commented Oct 7, 2020

Thanks for all your work, looks great!

Just a couple of very high-level comments:

  • The installation is very short and it is not a tutorial. I think it should just go on the landing page, as a subsection? In the installation re conda: "please follow the installation instructions of its user guide." -> ""please follow conda's installation instructions."

  • The section is called "Tutorials"; we do not need "Tutorial" in the subsection headers.

  • I'd suggest avoiding the word "interface". Does not mean much to most people who will need the tutorials. Maybe

    • Basic Usage
    • Advanced Usage
    • Changing Parameters of the Taxes and Transfers System
    • Changing Functions of the Taxes and Transfers System
    • Visualizing the taxes and transfers system (plural!)
    • Troubleshooting

    Please watch out for consistent Titlecase / lowercase everywhere, I do not have a strong opinion other than being consistent.

  • Parameters: "reformed parameter" does not sound like correct English to me. And it is about the whole system including everything else.

  • Functions: does it help to add ";" to the end of the plot_dag() lines so the plot id's are not shown?

  • Visualization: Is this still a Tutorial or already a How-To Guide now that some of it is in the Advanced Usage? I guess one could make a case for both, given the imbalance of Tutorials/How-To's I'd have a preference for How-To ATM.

  • Debugging / Debug mode with an exception: Example needs a cross reference to a place where this construct of user functions is explained. Most users will not understand what is going on in the example. Also, the description of which columns are returned could be more verbose, e.g., why it is useful? (you will be able to see all inputs to the function(s) that fail.

@amageh
Copy link
Member Author

amageh commented Oct 19, 2020

Thank you very much for the feedback @hmgaudecker ! I think we have addressed all of the comments (feel free to point out if we have missed anything).

One last open point concerns the debgugging/troubleshooting tutorial which we hadn't previously worked on. I'm not sure why, but the debug mode, or rather the example we have in the tutorial, does not seem to be throwing the warning or returning only a subset of variables anymore. Maybe someone who has more knowledge of the internals and recent changes than me can weigh in @MaxBlesch @ChristianZimpelmann ?

I have pushed my run of the tutorial (I updated some function arguments since they have changed since the tutorial was published) now, so you can compare to the old version here in the main documentation.

@amageh
Copy link
Member Author

amageh commented Oct 20, 2020

Nvm my last comment, it's working. Thanks @MaxBlesch !

Copy link
Collaborator

@hmgaudecker hmgaudecker left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

Just out of scope of this one, but I think we should keep it in there anyhow is what to do with the crosswalk. That one should not be placed as prominently anymore, if we want to keep it in there, we want to hide it in some section "compatibility" or so, but I do not have great ideas at present.

https://gettsim--232.org.readthedocs.build/en/232/functions.html# sort of replaces it for the output columns, that we should document a bit better. Maybe have a page with

Data columns

  • Basic user inputs
  • Typical outputs
  • All potential outputs

@MaxBlesch, could you make a suggestion on that one? Feel free to just open a new issue and/or PR if you think that will grow too large (having written this, it already feels like that), I do not have enough bandwidth these days to fully think through it.

@MaxBlesch
Copy link
Collaborator

Yes. I already work on something like that.
https://gettsim--230.org.readthedocs.build/en/230/variables.html has all basic inputs. I had the idea to extend that here to our default targets.

But all potential outputs are given by the documentation of functions. Once there are complete docstrings. That's the documentation we want to have?!

In any case out of scope of this PR. Very nice work!

@hmgaudecker
Copy link
Collaborator

https://gettsim--230.org.readthedocs.build/en/230/variables.html has all basic inputs. I had the idea to extend that here to our default targets.

But all potential outputs are given by the documentation of functions. Once there are complete docstrings. That's the documentation we want to have?!

Yes, excellent! In general, we will need think hard on how to package things, so that new users get the important bits at a glance without being overwhelmed by all the information there is in the background.

In any case out of scope of this PR. Very nice work!

Indeed, thanks @amageh, @davpahl, @Trichter33!

@hmgaudecker hmgaudecker merged commit 1e4f3e8 into master Oct 21, 2020
@hmgaudecker hmgaudecker deleted the tutorials branch October 21, 2020 07:18
@hmgaudecker hmgaudecker mentioned this pull request Oct 21, 2020
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants