Skip to content

Fixed issue with placeholder val of 1.0e18 - #44

Open
prkrtg wants to merge 1 commit into
mainfrom
pgupta/fp_constants_fix
Open

Fixed issue with placeholder val of 1.0e18#44
prkrtg wants to merge 1 commit into
mainfrom
pgupta/fp_constants_fix

Conversation

@prkrtg

@prkrtg prkrtg commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Not all constants were being generated properly in STATES, I think this was due to the placeholder base value -1.0e18 was outside the range where 64-bit floats can represent integer offsets exactly

Seemed to cause a bug where all rounded to the identical float value, so every new constant silently overwrote the previous one's entry in the reverse-lookup dict. Only the last one registered survived and got printed everywhere it is used

@weatherhead99 weatherhead99 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just checked this and it appear to work.

But I'm not sure about the functioning - it should be allowed to have two differently named floating point constnats with the same value and not have them ambiguous. For example, what if I want both science parallel and science serial high to be +11.0. I do not want science serial subbed into every place where I meant science paarallel.

I just tested that case and it doesn't appear to fail, though. So I'm approving this out of necessity of using it immediately. Thanks!

@prkrtg

prkrtg commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

I just checked this and it appear to work.

But I'm not sure about the functioning - it should be allowed to have two differently named floating point constnats with the same value and not have them ambiguous. For example, what if I want both science parallel and science serial high to be +11.0. I do not want science serial subbed into every place where I meant science paarallel.

I just tested that case and it doesn't appear to fail, though. So I'm approving this out of necessity of using it immediately. Thanks!

constLevels in wavgen.py is keyed on the constant's name (the string "sci_llel_low" vs "sci_tg_low"), not on its numeric value

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.

2 participants