bradbell / at_cascade

Cascading Dismod_at Analysis From Parent To Child Regions
https://at-cascade.readthedocs.io
4 stars 3 forks source link

Support prediction from priors #17

Open garland-culbreth opened 2 months ago

garland-culbreth commented 2 months ago

New PR building from #16.

Changes

Planned further changes

bradbell commented 2 months ago

I Verified that check_all.sh runs

Try running

python3 example/csv/prior_pred.py

I see some error messages when I do this.

bradbell commented 1 month ago

bin/check_all.sh ran to OK before and after merging the current master.

The following command

python example/csv/prior_pred.py

had the following in its output (even though it ends by printing prior_pred.py OK):

                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/bradbell/prefix/dismod_at/lib/python3.12/site-packages/dismod_at/get_name_type.py", line 74, in get_name_type
    assert False, msg
           ^^^^^

I suspect the real problem here is that you just commented out my checks at the end to make sure the values are correct, instead of changing it to do a proper check that the priors are predicted correctly and that is why prior_pred thinks it succeeded (and it probably did not).

ntemiq commented 1 month ago

To surface the latest feedback that my last two commits address:

I think that we should remove breaking up the computation from this example. The following example demonstrates that https://at-cascade.readthedocs.io/csv.break_fit_pred.html

No reason to add that complexity to example/csv/prior_pred.py

P.S.

As a first step I set breakup_computation to False (and got an error). Once that is fixed, the next step would be to remove the code that does not get used when breakup_computation is False.

P.P.S. I tried running bin/run_xrst.sh and got an error (it would also come up during bin/check_all.sh).

@bradbell the latest updates on this branch address your feedback - simplifying example/csv/prior_pred.py and I am now able to successfully run bin/check_all.sh

bradbell commented 1 month ago

This is closely related to the request for a wish list item that does not refit nodes with no data; see https://github.com/bradbell/at_cascade/pull/22#issuecomment-2292365338

bradbell commented 1 month ago

bin/check_all.sh generates the following output on my system:

at_cascade.git>bin/check_all.sh
bin/check_copy.sh
bin/check_copy.sh: OK
bin/check_invisible.sh
check_invisible.sh: OK
bin/check_tab.sh
check_tab.sh: OK
bin/check_version.sh
check_version.sh OK
bin/run_xrst.sh
xrst --local_toc --target html --html_theme sphinx_rtd_theme --index_page_name at_cascade 

warning: group_name = default
The following files have pages with the empty group name
but they are not in any xrst_toc commands starting at the root_file for the default group
   example/csv/prior_pred.py
Using following input_files: git ls-files
sphinx-build -b html -j 1 build/rst build/html
rm -r build/html/_sources
cp -r build/rst/_sources build/html/_sources
xrst: See warnings above
run_xrst.sh: aboring due to xrst errors above
at_cascade.git>

Note that it does not say check_all.sh: OK at the end. Also note that the error can be reproduced (and tested for) using the sub-command

at_cascade.git>bin/run_xrst.sh
xrst --local_toc --target html --html_theme sphinx_rtd_theme --index_page_name at_cascade 

warning: group_name = default
The following files have pages with the empty group name
but they are not in any xrst_toc commands starting at the root_file for the default group
   example/csv/prior_pred.py
Using following input_files: git ls-files
sphinx-build -b html -j 1 build/rst build/html
rm -r build/html/_sources
cp -r build/rst/_sources build/html/_sources
xrst: See warnings above
run_xrst.sh: aboring due to xrst errors above

This error can be fixed by adding the new example to the example/example.xsrt file. This will include the example in the following section: https://at-cascade.readthedocs.io/example.html

Also note that you will get some errors that need fixing in the example documentation.

bradbell commented 1 month ago

see https://github.com/bradbell/at_cascade/pull/22#issuecomment-2292365338

ntemiq commented 1 month ago

Note that it does not say check_all.sh: OK at the end. Also note that the error can be reproduced (and tested for) using the sub-command

My output from bin/check_all.sh ends with

split_covariate: OK
run_test.sh: OK
bin/run_test.sh test/table_exists.py
table_exists: OK
run_test.sh: OK
check_all.sh: OK

I just ran which xrst in my virtual env and got xrst not found - i hadn't realized the xrst check had been skipped. I'll install and fix the test

ntemiq commented 1 month ago

When running bin/check_all.sh, I get the following errors that appear to be related to spelling in files unrelated to the changes in this PR.

bin/check_all.sh 
bin/check_copy.sh
bin/check_copy.sh: OK
bin/check_invisible.sh
check_invisible.sh: OK
bin/check_tab.sh
check_tab.sh: OK
bin/check_version.sh
check_version.sh OK
bin/run_xrst.sh
xrst --local_toc --target html --html_theme sphinx_rtd_theme --index_page_name at_cascade 

warning: file = xrst/release_notes.xrst, page = 2024
spelling = pre, suggest = are, line 100
spelling = pre, suggest = are, line 103
spelling = relrisk, line 153
spelling = pre, suggest = are, line 270
spelling = pre, suggest = are, line 401

warning: file = xrst/release_notes.xrst, page = 2023
spelling = py, suggest = my, line 690
spelling = cov, suggest = cop, line 917
spelling = py, suggest = my, line 917
spelling = cov, suggest = cop, line 969
spelling = cov, suggest = cop, line 983

warning: file = xrst/release_notes.xrst, page = 2022
spelling = avgint, line 1205
spelling = avgint, line 1270
spelling = cov, suggest = cop, line 1405

warning: file = at_cascade/create_shift_db.py, page = create_shift_db
spelling = avgint, line 47
spelling = avgint, line 56
spelling = avgint, line 63

You can stop the spelling warnings using the spell command.
You also could add some of the following words to the project_dictionary:
   avgint   cov   pre   py   relrisk

Using following input_files: git ls-files
sphinx-build -b html -j 1 build/rst build/html
rm -r build/html/_sources
cp -r build/rst/_sources build/html/_sources
xrst: See warnings above
run_xrst.sh: aboring due to xrst errors above
bradbell commented 1 month ago
at_cascade.git>xrst --version
2024.6.25
at_cascade.git>bin/run_xrst.sh
xrst --local_toc --target html --html_theme sphinx_rtd_theme --index_page_name at_cascade 
Using following input_files: git ls-files
sphinx-build -b html -j 1 build/rst build/html
rm -r build/html/_sources
cp -r build/rst/_sources build/html/_sources
xrst: OK
run_xrst.sh: OK

If you looik at spell_package on https://xrst.readthedocs.io/latest/config_file.html#spell-package and the at_cascade xrst.toml file you will see that we are using the default spelling package. What do you get when you excecute

at_cascade.git>pip list | grep pyspellchecker
pyspellchecker                0.8.1

If your version is higher than mine, I will upgrade. If yours is less, you should upgrade.

ntemiq commented 1 month ago

i also get 0.8.1

❯ pip list | grep pyspellchecker
pyspellchecker                0.8.1
bradbell commented 1 month ago

How about xrst --version ?

ntemiq commented 1 month ago

That looks like the problem - I appear to be on a different xrst version

xrst --version
2024.0.2
bradbell commented 1 month ago

Look at install testing version https://xrst.readthedocs.io/latest/user-guide.html#install-testing-version

I have made some improvements to the spell checker so that it recognizes words that are used for page names (and other similar things).

bradbell commented 1 month ago

There is a way for xrst to update all your spelling commands (that is used when the dictionary or xrst changes). This is key so that xrst and the dictionaries can improve and one does not have to by hand edit all the spelling commands. See --replace_spell_commands on https://xrst.readthedocs.io/latest/run_xrst.html#replace-spell-commands

ntemiq commented 1 month ago

Thank you. I bookmarked that replace_spell_commands page - I need to get more acquainted with the xrst package in general. This latest push now passes bin/check_all.sh including the xrst checks.