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

[ENHC] Use tabular data format for sites #110

Open
lgolston opened this issue Sep 1, 2023 · 1 comment
Open

[ENHC] Use tabular data format for sites #110

lgolston opened this issue Sep 1, 2023 · 1 comment
Labels
enhancement New feature or request

Comments

@lgolston
Copy link
Contributor

lgolston commented Sep 1, 2023

Requested Feature/Enhancement

A key variable is 'sites', which is a list containing sites (dict format), which themselves contains a list of leaks (dict format). It is proposed that this variable be broken into a flat tabular data format - one variable for the sites and one for the leaks.

Need for feature/Enhancement

The code for LDAR-Sim contains extensive, often nested, for loops which is unusual in Python. This is due in part due the data not being in an array or table format.

Problem to solve

I suspect this change will significantly increase performance and readability since it will be possible to use vectorized functions and not always have to loop through the complex data structure.

Implementation Ideas

While the change is definitely non-trivial, the sites and leaks already contains a table-like format, linked by facility ID. I think a typical pandas dataframe format would work, or even long-term an embedded sqlite database.

Additional context

A motivating example is update_state, which seems to rebuild self.active_leaks from scratch at every time step with a nested loop:
https://github.com/LDAR-Sim/LDAR_Sim/blob/336b2ba5264459e072465b05faf6f7813cd1470f/LDAR_Sim/src/ldar_sim.py#L290C12-L290C12

@lgolston lgolston added the enhancement New feature or request label Sep 1, 2023
@ThomasGalesloot
Copy link
Contributor

Thanks for the great feedback. Updating the LDAR-Sim data structure is something we've wanted to do for some time now. We plan on addressing this in the near future, but it won't happen right away due to the non-trivial nature of developing and testing such a change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants