Bug - Fixed Divide by zero error eating quantum taco/ consuming Astral Energy Drink

[fronobulax]

I don't think we have any issues because of ambiguity.

Any program that does division without checking for a zero denominator or doing its best to guarantee a non-zero denominator is flawed at least in Ye Olden Days when numerical analysis was my bread and butter.

Fixing that makes the problem go away. The PR has not been "released" because the concoction data is different when the tests are run in isolation and in the test suite. I am claiming that is because TCRS tests leak the state of the consumables database but I cannot yet prove it to my satisfaction. I am working on that.

 

[MCroft]

Are we sure? The error with div/0 was happening with the astral energy drink in the real world experience in the start of this thread, despite the two AED’s each having a value for SpleenHit.

I don’t really know why we look up the item by name when usesRemaning requires an itemId anyway

 

[fronobulax]

I am sure. Look at the test for https://github.com/kolmafia/kolmafia/pull/1759.

If there is benefit to also testing the pure string "astral energy drink" then we can add it but how does that string get to the code?

I understand the following to mean the ambiguity has been dealt with prior to the computation.

Why is "astral energy drink" getting passed anyway? If we know what you want to consume, we know what the ID is, so we should be able to pass the correct item.

As I said on the PR there are two issues.

One is the question - is the test adequate for the divide by zero cases. If someone thinks that is not the case then new tests need to be written and added.

The other is that TCRS tests leak and I had to disable them to get the divide test to work.

 

[fronobulax]

Added

assertThat(ConsumablesDatabase.getRawSpleenHit("[5140]astral energy drink"), is(8));
assertThat(ConsumablesDatabase.getRawSpleenHit("[10883]astral energy drink"), is(5));
assertThat(ConsumablesDatabase.getRawSpleenHit("astral energy drink"), is(0));

I'm also not seeing anyplace where the non-test code calls ConsumablesDatabase.getRawSpleenHit with anything other than a concoction name so I feel like there is a lot of concern about something that never happens outside of an artificially created test.

Maybe we are in different places because as far as I am concerned I have already fixed the divide problem?

 

[fronobulax]

I think this is done.

Fix Divide by zero consumption errors. by jaadams5 · Pull Request #1759 · kolmafia/kolmafia

Failing tests to find and fix divide by zero for some consumption. See https://kolmafia.us/threads/divide-by-zero-error-eating-quantum-taco-consuming-astral-energy-drink.28941/

github.com

 

[MCroft]

I haven’t reviewed the code (still at work). Does this solve Adventuristo’s original problem with a scripted use of the Astral Energy Drink?

 

[fronobulax]

I haven’t reviewed the code (still at work). Does this solve Adventuristo’s original problem with a scripted use of the Astral Energy Drink?

I believe so unless my belief that "ash chew" leads to a UseItemRequest is incorrect.

I started with a failing test that failed because of a divide by zero and while there have been some distractions along the way the test now passes

 

[Veracity]

I believe so unless my belief that "ash chew" leads to a UseItemRequest is incorrect.

It calls the "chew" CLI command
which calls UseItemRequest.getInstance(ConsumptionType.SPLEEN, item)
which returns a SpleenItemRequest(item).

 

[fronobulax]

For reference the Feb. 23 2023 change is https://github.com/kolmafia/kolmafi...1b1a46432a3cab43e8e8c6f02e6aadd395b543db40315

but it corrected the item id of the "new" AED form 10882 to 10883.

 

[fronobulax]

r27421

 

[fractalnavel]

Thanks for the fix!

My first clue that there was something wrong was today when my daily script tried to drink 196 Schrödinger's thermoses. It had been acquiring but failing to drink for that long. Near the end of the script, so missed by my inattention.

 

[fronobulax]

Thanks for the fix!

My first clue that there was something wrong was today when my daily script tried to drink 196 Schrödinger's thermoses. It had been acquiring but failing to drink for that long. Near the end of the script, so missed by my inattention.

Just for my piece of mind you are reporting that things work now and had been broken for perhaps 196 days?

Fixing this was a slog, some of that self-inflicted, and I don't wish to revisit it immediately

 

[fractalnavel]

Just for my piece of mind you are reporting that things work now and had been broken for perhaps 196 days?

Fixing this was a slog, some of that self-inflicted, and I don't wish to revisit it immediately

Sorry, yes, it works now : )