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

Duplicate MUSICA name keys are not detected #235

Open
K20shores opened this issue Sep 9, 2024 · 4 comments · May be fixed by #241
Open

Duplicate MUSICA name keys are not detected #235

K20shores opened this issue Sep 9, 2024 · 4 comments · May be fixed by #241
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@K20shores
Copy link
Collaborator

@SancyW reported a configuration to use which was failing in music box interactive. The configuration is listed below. The issue is that there were 6 reactions which were actually 3 reactions, each duplicated. Their musica names are

  • LOSS_FORM_depo
  • LOSS_NO2_depo
  • LOSS_FORM_washout

configuration.zip

Reproduce

To see the incorrect behavior, download and unzip the configuraiton and run music_box -c configuration/my_config.json -o output.csv

Observe that there is a key index error.

Then, open up configuration/camp_data/reactions.json and find the duplicated reactions above. Delete one of each pair and rerun the configuration. You'll see that it passes.

We should alert when we find duplicate keys and let the user know and then exit

Acceptance criteria

  • All reactions with duplicate musica names are detected
  • Duplicate reactions are printed to the terminal and the error is explicitly listed with instructions on how to fix them
  • A unit test is added that successfully detects duplicate keys

Ideas

@K20shores K20shores added bug Something isn't working good first issue Good for newcomers labels Sep 9, 2024
@SancyW
Copy link

SancyW commented Sep 9, 2024

Oh, yes, sorry, that was my mistake. It’s a good idea to add an alert to remind users. Thanks for that!

@mattldawson mattldawson added this to the Sancy's MusicBox Paper milestone Sep 9, 2024
@carl-drews carl-drews self-assigned this Sep 9, 2024
@carl-drews
Copy link
Collaborator

The run-time error looks like this:

2024-09-12 13:28:52 - DEBUG - main.main - Working directory = /home/drews/MusicBox/music-box-duplicate-keys-235/temp
2024-09-12 13:28:52 - DEBUG - main.main - Configuration file = configuration/my_config.json
Traceback (most recent call last):
File "/home/drews/.conda/envs/musicbox/bin/music_box", line 8, in
sys.exit(main())
^^^^^^
File "/home/drews/.conda/envs/musicbox/lib/python3.12/site-packages/acom_music_box/main.py", line 161, in main
result = myBox.solve(musicBoxOutputPath)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/drews/.conda/envs/musicbox/lib/python3.12/site-packages/acom_music_box/music_box.py", line 154, in solve
ordered_rate_constants = self.order_reaction_rates(
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/drews/.conda/envs/musicbox/lib/python3.12/site-packages/acom_music_box/music_box.py", line 325, in order_reaction_rates
ordered_rate_constants[rate_constant_ordering[key]] = float(value)
~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
IndexError: list assignment index out of range

@carl-drews
Copy link
Collaborator

The branch is here:
main...duplicate-keys-235

Not completed yet - still need to add the unit test.

@carl-drews
Copy link
Collaborator

Pull request is:
#241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants