Skip to content
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

fix: backward dispersion #1

Merged
merged 2 commits into from
Aug 18, 2023
Merged

fix: backward dispersion #1

merged 2 commits into from
Aug 18, 2023

Conversation

AlFontal
Copy link
Owner

This addresses rich-iannone#81, and also could be considered to address rich-iannone#66.

Basically, there were 2 really small errors that made the logic of running backwards dispersion models not work.

For starters, the run_model function has a hardcoded 'forward' parameter, so no matter the direction specified in add_dispersion_params, the hysplit_dispersion function always received 'forward' as the parameter. This is fixed
in commit caead2f.

Second, the add_dispersion_params function has no duration parameter. The value of duration that is passed onto hysplit_dispersion is computed by the difference in hours between start_time and end_time:

https://github.com/rich-iannone/splitr/blob/44851122514e1296291f0e9b082326717549715f/R/run_model.R#L51

In a backwards model, the end_time is before the start_time, so this variable takes negative values.

When writing the CONTROL file, however, there is an ifelse clause that prepends a '-' to the duration value. In the case of an already negative value, this writes a line with --duration, leading to a faulty CONTROL file that HYSPLIT doesn't accept and it just doesn't run. Removing the ifelse clause seems to work and is what I did in d4ca419.

The duration is computed as the difference between the start and end
times. If the start time is after the end time, the duration is already
negative, so we don't need to prepend a "-" to it or we have a double
negative before the duration value and HYSPLIT crashes.
@AlFontal AlFontal merged commit b6e0dbc into main Aug 18, 2023
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.

1 participant