NAVADMC / ADSM

A simulation of disease spread in livestock populations. Includes detection and containment simulation.
Other
10 stars 5 forks source link

Vaccination Rings - 2 fields are filled instead of one field #691

Closed missyschoenbaum closed 6 years ago

missyschoenbaum commented 8 years ago

When I create a new ring, I am presented with the ability to select a “triggering” production type, a “recipient of vaccination” production type, and then the radius of an external ring and an internal ring (optional).

When I mouse over to select what I am thinking is the triggering production type, the selection is populated into both triggering and into recipient. I asked TIm if this would be a preferred action, and he did think so. I am thinking that both of these boxes must be selected somehow in the background.

What I would expect is for the focus to be set on the first box, and allow the click on production type to populate it, and then to select the second box.

rings2ptappear

josiahseaman commented 8 years ago
$('.production-type-list').first().addClass('focus')

$(document).on('.production-type-list', 'click', function(){
    $('.production-type-list.focus').removeClass('focus')
    $(this).addClass('focus')
})
josiahseaman commented 8 years ago

I noticed this issue is assigned to @ndh2 so I'll leave it alone. Hopefully the above code is a good hint, though it's just typed from memory, so it'll need modification. Essentially, there's already some focus pulling mechanism on click, but it doesn't run at start, which is what the first line is for.

missyschoenbaum commented 7 years ago

This is still happening.

ConradSelig commented 6 years ago

I think the JavaScript may actually be working correctly on this one. My current suspicion is that the 'trigger group' and the 'target group' are actually the same group because of how they are setup in the Django Forms (they both are build using ProductionTypeList()). Looking into whether this hypothesis is correct and how to fix it. More updates to come.

ConradSelig commented 6 years ago

I was both correct and incorrect about my last post, here is the most recent update:

I was correct that it is not a JavaScript/HTML error, the forms are displaying what they are told - and are not the reason both fields are being filled. I was incorrect that the data contamination was being caused by the same function (ProducitonTypeList()) being used for both fields. There is a clear separation of data between the trigger group and the target group.

I am closer to the solution however, I can tell it's not a forms error. If I force data into the form, it displays like it's told, but it does not change the database. This means that the data crossover is happening somewhere between when the user add/removes a production type and when the database write occurs.

I'm going to continue to narrow down the bug, more updates to come.

missyschoenbaum commented 6 years ago

The database write happens when the apply button is used. I guess there is also a write to the settings.db that happens immediately? Be sure you are looking at the right one when you are checkin.

BryanHurst commented 6 years ago

After Conrad spent some time looking at this, I took a look as well. These two fields are tightly coupled currently and have no mechanism for uncoupling their input.

Getting these two to work independently would be a large undertaking.

@missyschoenbaum we will need to decide the priority on this one.

missyschoenbaum commented 6 years ago

@BryanHurst Let me chat with SMEs about it.

missyschoenbaum commented 6 years ago

@BryanHurst we need to get it fixed. We are trying to model the concept that you would only vaccinate long-lived production types when the other production types that are high disease spreaders.

Priority wise, let's drop it to the lowest of our release priorities. I rearrange #888 and put it in the nice to have group, realizing that it's probably not going to make it.

missyschoenbaum commented 6 years ago

I will add it to known bugs for release.

missyschoenbaum commented 6 years ago

@BryanHurst I was testing my work around. It is very easy to use the advance panel (django window) to make this change. It saves correctly, and I confirmed it saved by looking at the database in SQL editor. However, when I run this way (ring target and ring trigger not same production type), no vaccination happens. Any thoughts other options for work around?

BryanHurst commented 6 years ago

@missyschoenbaum I spent today getting this working UI side. I'm running late for another appointment, so if you have time, can you pull the latest version, check that the database state saves properly with the form and confirm that no vaccination happens still?

If we still aren't getting vaccination, we should ask Neil if there is another coupling of these fields (in the CEngine) that we should be aware of.

missyschoenbaum commented 6 years ago

Will do

missyschoenbaum commented 6 years ago

@BryanHurst still no vaccination after update.

BryanHurst commented 6 years ago

@ndh2 Do you know of any reason off the top of your head why having a different list of triggers and targets would cause no vaccination to happen?

The database is populating as expected with Cattle in Trigger, then Cattle and Swine in Target.

ndh2 commented 6 years ago

Can you post a file on Google Drive for me to look at?

BryanHurst commented 6 years ago

Sure, it is now in the 691 folder.

ndh2 commented 6 years ago

I just ran this scenario and I see units in the vaccine immune state. What outputs are you looking at that say no vaccination is happening?

I turned on the daily state table output and tested it from the command line with: main_loop/adsm_simulation --rng-seed 1 test691.db

BryanHurst commented 6 years ago

This may be related to a bug I found on #891 where 'use_vaccination' on ControlProtocol wasn't being saved correctly.

I'm going to move this one back to testing.
When opening a scenario, just double check that the ControlProtocol page is correctly detecting triggers and setting the use_vaccination param.

missyschoenbaum commented 6 years ago

OK, I will look at it again. I just looked at the output within the application (map and variable graphs).

missyschoenbaum commented 6 years ago

I did pull an update just to be sure. I ran 12 iterations, and output states files so I could check. I am seeing no vaccination in the application (map or variable graphs) and no change to a V state in the states output.

Within Control Protocols, The vaccination did appear to be checked, but not editable. The parameters within vaccination were filled. The Ring setup looked like it had a trigger and a different target.

missyschoenbaum commented 6 years ago

@BryanHurst I just went and delete the ring to make sure it would still vaccination with trigger and target the same. I am now unable to add a production type to either box. Want a different ticket?

missyschoenbaum commented 6 years ago

I started again from a clean sample, and it does vaccinate.

BryanHurst commented 6 years ago

So vaccination is working, but you are having an issue with trigger and target input?

missyschoenbaum commented 6 years ago

The current method, which binds the trigger and the target together vaccinates if I start from scratch.

I was attempting a work around using the django panel to change either the trigger or the target. I am able to do that easily, and confirm it is saved. However, when I run the sim this way, there is no vaccination that I can observe, even though it appears I have a valid set of parameters.

I may have a different problem that I am trying to sort out, where it clears the ring fields, and won't let me add anything. It need to figure out steps to recreate on this.

missyschoenbaum commented 6 years ago

OK, I think this was my mistake. I have a work around in place, so we can just go back to uncoupling fields.

BryanHurst commented 6 years ago

The boxes should be un-coupled at this point.

The new help text in the boxes describe the new steps for usage.
This new style of input is also used in other production type selection areas (like Vaccination Triggers)

BryanHurst commented 6 years ago

@missyschoenbaum can you confirm what still needs to be fixed on this one?

missyschoenbaum commented 6 years ago

OK, I will pull an update and check

BryanHurst commented 6 years ago

There shouldn't be any updates since the end of last week. But my last testing was showing this working as expected. I may need steps to replicate any issues.

It might be good to do today's testing with a fresh install/update and clean workspace.

missyschoenbaum commented 6 years ago

@BryanHurst Yes, working as expected. Looks great.

missyschoenbaum commented 6 years ago

I will retest when it pushes through to testing.

missyschoenbaum commented 6 years ago

Worked perfectly.