-
Notifications
You must be signed in to change notification settings - Fork 5
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
Seed data observations outside the simulation spatial domain cause extract operator error #5
Comments
The issue is not present if the province of Crete is not cropped from the data when running a default SVEIRD simulation. |
NA
/NaN
argument provided to [
(extract operator) when using default grc_ppp_2020_1km_Aggregated_UNadj
seed dataNA
/NaN
argument being provided to [
(extract operator) when using default grc_ppp_2020_1km_Aggregated_UNadj
seed data
|
The same issue occurs with the simulation options in #8, but cropping the default Congolese provinces. |
I will try that; determining if the aggregation factor is related may be important. |
The issue occurs with that aggregation factor as well. |
DiagnosisCropping the province containing the only seed data is the likely source of the issue. All the following locations are on the island of Crete:
Discussion@ashokkrish, in your email asking for me to prioritize this issue this week, you said
You wrote as if you intend for the simulation to be limited to the island of Crete, when what is happening when we crop this island is the simulation is running everywhere in Greece but Crete. The seed data is limited to Crete, so there is nothing for the modelling process to work with when Crete is cropped from the data. Related issuesWhy the issue #4 occurs when the simulation is set to Greece will be determined in #4. SolutionThe simulation can reject the seed data if there is any data point in it which occurs in a cropped province or state. An alternative is to crop such observations from the seed data, and also ensuring that the final seed data has a final length of at least one. ## Ensure there is at least one observation in the seed data.
stopifnot(length(filteredSeedData)) |
I don't know why @ashokkrish was able to run the simulation, cropping the island of Crete, in #4. Perhaps he used different seed data? |
I can replicate this issue when using this seed data COD_InitialSeedData_Beni.csv. Other options:
|
Without cropping the map / limiting the simulation to Nord Kivu—i.e. |
Yes it would run as long as a seed data is uploaded. The seed data may be for the whole country or selected set of cities. What's problematic is if the data is seeded outside the spatial domain. I assume the simulation runs even if you select only one province however the seed data is for two provinces. Correct? |
Without cropping it runs for the whole country for me. |
I used GRC_Crete_InitialSeedData.csv and I made sure I cropped Crete on the left panel. |
I encounter this issue's error when I run a simulation of Greece limited to Athos with the Crete data. Perhaps the issue is caused by the presence of data outside of the domain. More testing will be needed, because this is a difficult to locate error; maybe I'll have better luck running the application in showcase mode to see exactly what is running before the error is encountered; it happens very quickly, however. I'll check in on Shiny debugging methodology first. |
I said,
You replied to another thing I said,
It took me about fifteen minutes to definitively determine where the Manima housing development is, but I saw on the map that it's next to Mambasa and that's definitely within the Ituri province, so the seed data does cover two provinces. Running a simulation with the Beni seed data and limiting the simulation to either Ituri or Nord Kivu will cause this error, but if I include both provinces the simulation will run. ConclusionThe cause of this issue, determined through scientific deduction 🧑🔬 🧪, is the presence of seed data outside of the simulation domain. Any seed data outside of the simulation domain seems to be an issue. The solution is, then, as I said earlier but with the assumption that cropping worked in the opposite direction to how it does work in the application, to ensure that no point of seed data exists outside of the polygons. That won't be too difficult because the vector data of a coordinate and a polygon can definitely be quickly checked. We can ensure that all points of seed data exist within some polygon the simulation is including, and if not prevent the simulation from running. |
NA
/NaN
argument being provided to [
(extract operator) when using default grc_ppp_2020_1km_Aggregated_UNadj
seed data
I have wanted this kind of validation for a long time now (almost two years) but told to myself hopefully the user uploads the propre seed data. Yes, we will address in the coming updates. Just wanted to caution you about not getting too deep into this as I would like a stable version by this Friday to get my slides going. |
Indeed. I'll work on the feature in the unstable branch that I'll create. The stable branch (main) has now received the fixes for the other issues we discussed yesterday. These are the items on my checklist I wrote yesterday during our meeting which I have not yet worked on:
It's possible to refactor the UI code and make some edits to the server without upsetting the current logic (which would also resolve #18), but I'll leave that to the unstable branch just so that not too much changes in the code and your familiarity with it isn't undermined. |
Here are resources I'll be using to resolve this issue, which is in the point in polygon domain of spatial data analysis.
StackOverflow user Robert Hijmans provides four examples using the terra library. |
This is excellent. Please go ahead. |
@kle6951 and @Toby-exe, this issue can be resolved in stable as well. Please investigate the solution I wrote about in one of the last comments. I suggest working with the solution outside of the context of our Shiny app until you're comfortable with it. Then write a wrapping function to take a set of coordinates and a country code ( That way it can be used in a pipeline like the following
|
@ashokkrish please see the latest commit (61564e2) for progress on this issue |
I believe this issue can be closed as of 321ef2b |
While attempting to replicate the issue in #4 as described by Ashok in a personal communication, I encountered the following error (which causes the usual busy client UX).
The text was updated successfully, but these errors were encountered: