Skip to content

Conversation

jinningwang
Copy link
Member

#585

  • Fix ACEc.bias.tex_name
  • Change ACEc.bias.default to -1 to follow NERC's definition
  • Add references

@jinningwang
Copy link
Member Author

@cuihantao
One question regarding this PR.

As here I changed the default value and equation, it seems to be a potential breaking change. Before manually updating parameters of ACEc.bias, users can run into unexpected results.

So, I'd like your suggesstion that which option would be better for us?

  1. Revise the equations: use abs() to eliminate the impact of sign
  2. If there is a parameter correction step, correct positive values to negative

@cuihantao
Copy link
Collaborator

If it's always going to be a negative value, you can use "abs". This is to safeguard users who entered the value mistakenly as a positive value.

If both positive and negative values have a meaning, then we should do any correction to parameter but rather correct the equation.

@jinningwang
Copy link
Member Author

Bias and frequency response are both expressed as negative numbers. In other words, as frequency drops, MW output (β) or desired output (B) increases. Both are measured in MW/0.1 Hz

Given NERC's suggestion, I update the PR with first option.

@jinningwang
Copy link
Member Author

@cuihantao ready to merge

@cuihantao cuihantao merged commit e442d38 into CURENT:develop Dec 13, 2024
3 checks passed
@cuihantao
Copy link
Collaborator

Jinning, thanks!! I'll tag a new version shortly.

@jinningwang jinningwang deleted the bias branch April 24, 2025 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants