Skip to content

Generate source coverage#36

Open
davidrandell84 wants to merge 8 commits intosede-open:mainfrom
davidrandell84:generate_source_coverage
Open

Generate source coverage#36
davidrandell84 wants to merge 8 commits intosede-open:mainfrom
davidrandell84:generate_source_coverage

Conversation

@davidrandell84
Copy link
Copy Markdown
Collaborator

Description

Added function to source_model to be able to generate sources consistent with coverage map to avoid issues where initial random source gets screened out leaving no sources and the code crashing.

Function puts existing approach inside of a while loop repeatedly trying to propose source 100 times until at least one source is inside the coverage. If 100 proposals are reach the code will error (I think if this happens probably there is something fundamentally wrong with the input data).

The existing approach of generating sources is unaffected so this is not a breaking change just an improved approach if the user wants to use it.

Fixes # (issue)

Add function in source_model - generate_sources that will generate sources consistent with the coverage and is compatible with both Gaussian plume and Finite Volume
Refactored initialise_dispersion_model to be compatible with the new function and more general to cover the FE case.
Updated example notebooks to call new function where relevant instead of old method
Added test function to test new function.

Please delete options that are not relevant.

  • [ x ] Bug fix (non-breaking change which fixes an issue)
  • [ x ] New feature (non-breaking change which adds functionality)
  • [ x ] This change requires a documentation update

Jupyter Notebooks

Examples updated where sources were previously generating using random sampling unrelated to the coverage map. I've not changed "true" source model generation.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • [ x ] New test test_generate_sources. Calls generate sources checks sources are inside site limits and that running the screening afterwards doesn't change the number of sources.

Checklist:

  • [ x ] My code follows the style guidelines of this project
  • [ x ] I have performed a self-review of my code
  • [ x ] I have made corresponding changes to the documentation
  • [ x ] My changes generate no new warnings
  • [ x ] I have added tests that prove my fix is effective or that my feature works
  • [ x ] New and existing unit tests pass locally with my changes
  • [ x ] Any dependent changes have been merged and published in downstream modules

@jallsop2
Copy link
Copy Markdown

jallsop2 commented Mar 6, 2026

Based on our conversation last week, it would be helpful for the error messages to say which source model was the cause of the problem when running pyELQ with multiple source maps. I guess this would be done in ELQModel.initialise, since that's where the source models have corresponding keys. If the key was at the beginning of the error message that would be helpful for filtering out source models that are causing problems in a run.

@davidrandell84
Copy link
Copy Markdown
Collaborator Author

Based on our conversation last week, it would be helpful for the error messages to say which source model was the cause of the problem when running pyELQ with multiple source maps. I guess this would be done in ELQModel.initialise, since that's where the source models have corresponding keys. If the key was at the beginning of the error message that would be helpful for filtering out source models that are causing problems in a run.

I've updated the error message in generate_sources to include the source map label. I think this will achieve what you are asking for if you have multiple source maps.

I also cut the number of iterations of the while loop to 50 since you were concerned about runtime. I could maybe make that a user defined variable?

@davidrandell84 davidrandell84 reopened this Mar 9, 2026
@jallsop2
Copy link
Copy Markdown

I've updated the error message in generate_sources to include the source map label. I think this will achieve what you are asking for if you have multiple source maps.

I also cut the number of iterations of the while loop to 50 since you were concerned about runtime. I could maybe make that a user defined variable?

Yes, that will work. I forgot that the source models have a label within them. I have tested the solution and it only took about 2 seconds to generate the sources 50 times, which is less than I expected. Since it's quite negligible I think there's no real reason for the user to want to change it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants