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

Inconsistent equality comparisons between where and expression sections #728

Open
1 of 3 tasks
irm-codebase opened this issue Dec 12, 2024 · 2 comments
Open
1 of 3 tasks
Labels

Comments

@irm-codebase
Copy link
Contributor

What happened?

This bug was noticed while implementing #704

We are incorrectly using = instead of == for equality comparisons in where: strings.

This is confusing (= is generally used for assignation, not comparison).

Here is an example from the balance_storage constraint.

foreach:
- nodes
- techs
- timesteps

# '=' used for comparison (include_storage == true, base_tech==supply, base_tech==demand)
where: (include_storage=true or base_tech=storage) AND NOT (base_tech=supply OR base_tech=demand)
equations:
 # both '=' and '==' are used correctly
- expression: "storage == $storage_previous_step - sum(flow_out_inc_eff, over=carriers) + sum(flow_in_inc_eff, over=carriers)"

sub_expressions:
  storage_previous_step:
  # '=' used for comparison (timesteps == ... and cyclic_storage == ...)
  - where: timesteps=get_val_at_index(timesteps=0) AND NOT cyclic_storage=True
    expression: storage_initial * storage_cap
  # ditto
  - where: "(\n  (timesteps=get_val_at_index(timesteps=0) AND cyclic_storage=True)\n\
      \  OR NOT timesteps=get_val_at_index(timesteps=0)\n) AND NOT cluster_first_timestep=True"
    expression: (1 - storage_loss) ** roll(timestep_resolution, timesteps=1) * roll(storage,
      timesteps=1)
  # ditto
  - where: cluster_first_timestep=True AND NOT (timesteps=get_val_at_index(timesteps=0)
      AND NOT cyclic_storage=True)
    expression: (1 - storage_loss) ** select_from_lookup_arrays(timestep_resolution,
      timesteps=lookup_cluster_last_timestep) * select_from_lookup_arrays(storage,
      timesteps=lookup_cluster_last_timestep)

Which operating systems have you used?

  • macOS
  • Windows
  • Linux

Version

v0.7.0

Relevant log output

No response

@irm-codebase irm-codebase added bug v0.7 (upcoming) version 0.7 labels Dec 12, 2024
@brynpickering
Copy link
Member

Not really a bug, although I can see why one would prefer to be consistent across the where and expression strings. We're just not strict about being pythonic in our math, because we're not targetting heavy Python users, necessarily.

@irm-codebase
Copy link
Contributor Author

I did not really meant it as striving to be pythonic (a = b is assignation in all programming languages I know tbh), but more about the consistency and readability of the YAML, and to avoid possible bugs (two distinct operations sharing the same symbol spells trouble).

I am not an expert in our parsing methods, but this seems like an easy enough fix to me.

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

No branches or pull requests

2 participants