PolicyEngine / PolicyEngine/policyengine-us

Consider migrating from mask approach to np.divide for division by zero handling

Open
#6,356 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
162
Forks
212
Avg merge
3d 3h
Merged PRs (30d)
97

Description

## Summary

PolicyEngine currently uses a mask approach to handle division by zero and avoid numpy warnings. However, `np.divide` with `out` and `where` parameters provides a cleaner alternative that's already used in some parts of the codebase.

## Current Approach (Mask)

```python
effective_rate = np.zeros_like(income)
mask = income != 0
effective_rate[mask] = tax[mask] / income[mask]
```

## Proposed Approach (np.divide)

```python
effective_rate = np.divide(
tax,
income,
out=np.zeros_like(income),
where=income != 0
)
```

## Analysis

I've created a reproducible comparison: https://gist.github.com/MaxGhenis/74678e11c0fa830fa9c18924e362f24f

Key findings:
- Both approaches avoid division by zero warnings ✅
- Both produce identical results ✅
- `np.divide` reduces runtime by ~58%
- `np.divide` is more concise (single expression vs 3 lines)
- `np.divide` is already used in `weekly_hours_worked.py`

## Migration Path

1. Update style guide to recommend `np.divide`
2. Gradually migrate existing mask usage when touching those files
3. Use `np.divide` for all new code

## Questions

- Are there any edge cases where the mask approach is preferred?
- Should we do a bulk migration or gradual replacement?

cc @nikhilwoodruff

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.