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

339 add support for converting pgd from pga for the earthquake controller #342

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

navarroc
Copy link
Member

@navarroc navarroc commented Jan 8, 2025

Added a conditional to check if the requested hazard is PGD and if it's not directly supported by the attenuation, it calls an internal method that handles converting PGA to PGD (as long as the hazard supports PGA).

I also added a unit test to verify it's working correctly.

Copy link
Member

@longshuicy longshuicy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. Ran the test and all passed.

@@ -336,11 +336,13 @@ public static double getCorrectUnitsOfPGA(double pga, String units0, String unit
public static double getCorrectUnitsOfPGD(double pgd, String units0, String units1) {
if (units0 != null && units0.equalsIgnoreCase(units1)) {
return pgd;
} else if (units_m.equalsIgnoreCase(units0) || "m".equalsIgnoreCase(units0) && units_ft.equalsIgnoreCase(units1)) {
} else if (units_m.equalsIgnoreCase(units0) || "m".equalsIgnoreCase(units0) && (units_ft.equalsIgnoreCase(units1) || "ft".equalsIgnoreCase(units1))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because we have inconsistent definition of feet vs ft?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there is some inconsistency in the static variable definitions (some are abbreviations and some aren't). Also, I wanted to match that a user could request "m" or "meters" so I made it so a user could request "feet" or with the abbreviation "ft".

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.

Add support for converting PGD from PGA for the earthquake controller
2 participants