smallAreaHealthStatisticsUnit / rapidInquiryFacility

The Rapid Inquiry Facility (RIF) helps epidemiologists and public health researchers in environmental health activities.
GNU Lesser General Public License v3.0
14 stars 5 forks source link

Risk analysis selection at high resolution #101

Closed peterhambly closed 5 years ago

peterhambly commented 5 years ago

This also include a first restructure of the areamap code.

This is a partial implementation of issue #78 (Risk Analysis selection at high resolution (e.g. MSOA) does not perform acceptably); the conversion of geoJSON tiles to PNG tiles by the middleware has not yet been implemented, At high resolutions (>5000 areas) selection by clicking on the area is not permitted and restore of saved study selections only show the selected area (the PNG tile map will go below).

The RIF will now work at COA level with fully 64 bit browsers (Firefox and Edge). Browsers limited to 32 bit address space (all other Windows browser) will crash when memory exceeds 2GB.

Testing showed up issues with Risk Analysis bands so these were fixed and full support for risk analysis added at the time.

You must re-run alter_10.sql

cd rifDatabase/Postgres/psql_scripts
psql -d sahsuland -U rif40 -f alter_scripts/v4_0_alter_10.sql

Also included:

Further changes are needed to support risk analysis:

devilgate commented 5 years ago

It's unfortunate that you've changed Adj_Cov_Smooth*.R and SmoothResultsSubmissionStep.java just at the same time that I have:

and all in a different branch.

Oh well. The best thing now is for me to merge this branch into mine and tidy up manually.

Other than that, I don't see any obvious problems with it.

devilgate commented 5 years ago

The merge went surprisingly smoothly. Git is able to track your changes through to my renames, by the look of it, which is great. But there's something not quite right with the SQL script. It seems to do part and then just hang. I have it looking like this now:

screenshot 2018-10-11 at 16 17 25

Hangs till I hit Ctrl+C, and then says:

^CCancel request sent
psql:alter_scripts/v4_0_alter_10.sql:155: ERROR:  canceling statement due to user request
CONTEXT:  SQL statement "ALTER TABLE t_rif40_studies ADD COLUMN select_state JSON NULL"
PL/pgSQL function inline_code_block line 6 at SQL statement
Time: 182533.724 ms (03:02.534)
devilgate commented 5 years ago

This morning I could run a disease mapping study fine. And a risk analysis would run but fail with a statistics problem that I was going to look at this afternoon.

Now, after the merge, I can still run a disease mapping OK. But risk analyses fail, saying "Error: Study 588 - TEST 1005 failed to be extracted." Which I've never seen before. From the log it appears to be an SQL problem, so I'm guessing it's because the script didn't run properly.

If you want to take a look, the branch is issue-#51_integrate_risk_analysis_script

peterhambly commented 5 years ago

Postgres needs a lock on the table. Log out from psql, pgadmin3 and shutdown tomcat Peter

peterhambly commented 5 years ago

This is almost certainly caused by the band_id code which did not exist in the previous code (i.e. everything was a disease map). Also, the new code sets rif40_studies.study_type correctly this causes new paths in the extract in particular no area_id column in the results (map) table.


Email: phambly@fastmail.co.uk

Links:

  1. https://github.com/smallAreaHealthStatisticsUnit/rapidInquiryFacility/pull/101#issuecomment-429003527
  2. https://github.com/notifications/unsubscribe-auth/AGnHJaOKC0sF_5um1yzcSKfdgYHvAm-cks5uj2ZmgaJpZM4XXZ_0
devilgate commented 5 years ago

Ah! I should have thought of that. Thanks.

devilgate commented 5 years ago

This is almost certainly caused by the band_id code which did not exist in the previous code (i.e. everything was a disease map). Also, the new code sets rif40_studies.study_type correctly this causes new paths in the extract in particular no area_id column in the results (map) table.

So what's the next step to fix this? I guess we need database changes?

Also we're discussing this in the wrong place. This current PR can be merged, as far as I'm concerned.