Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
313 stars 134 forks source link

Reset values of Specific Heat #719

Closed Jazzbeerman closed 1 year ago

Jazzbeerman commented 1 year ago

Hello, I would like to say a huge thank you for your program! It seems to me the most comfortable compared to others, and it seems that my purchase of BeerSmith was in vain)) But I would like to point out one error that appears to me regularly. I'm using a regular 50L stainless steel pot that I converted into a mash tun. I want to enter my equipment data, but when I enter 0.12 in Specific Heat it drops to 0 after I save it. After experimenting, I managed to find out. that the program changes the comma character to a dot on its own. The first save is successful. If you make any changes to other parameters, or press the save button again, then Specific Heat is reset to zero again. The problem may not be that big, however, I use homemade malt, which means I need more temperature breaks. When designing my mash profile, the effect of heat capacity definitely matters. However, by the time the mash profile was saved, this parameter had already reset to zero, in connection with which I noticed that the program recommends using less mash for the decoction mash method that I use. The difference compared to BeerSmith is about 1 liter. It seems to me that the problem is in this parameter. Yes, it would be possible to count in BeerSmith, however, for many reasons, I would like to refuse it.

matty0ung commented 1 year ago

Hi there, Glad you're finding the program useful. Sorry to hear about the problems. Generally, there's an issue for all software in deciding how dot (.) and comma (,) are used in numbers. Eg, one thousand and a quarter is written in decimal as 1,000.25 in the USA but as 1.000,25 or 1 000,25 in France. Ideally, we should either detect the locale and work out which format to use or allow you to specify which you prefer. Generally we try to do the former, but I can see something has gone wrong somewhere because I'm getting USA-format numbers on my French-locale laptop. I will take a look and see if I can find out what is going wrong. In the meantime, I don't know if it solves your problem, but, as a temporary workaround, try 0.12 instead of 0,12. (If that doesn't work, let us know, because it might mean there is a second bug!)

matty0ung commented 1 year ago

Hmm, OK, so a bit more experimenting and it could be that I'm seeing different things as I have an odd set-up on my laptop. (It's a bit of a mixture of locales.) So perhaps the thing you are seeing is indeed a separate issue. On the other hand, so far I cannot reproduce the problem on my machine.

To help me track down what's happening, could I trouble you for some additional info:

Jazzbeerman commented 1 year ago

Thanks for the prompt response! I will answer in order. Mistakes were met in some places, but I don’t always treat them as important. Well, for example, if you change the date of brewing beer, then I have a window displayed from Monday to Thursday. But using the arrows on the keyboard, you can rewind to other days of the week. A trifle that does not affect the work in any way. Or, for example, when adding yeast that is not in the base, there is no window in which the weight in one bag is indicated. Again, a trifle that is not particularly important. (I want to say that I see minor errors, but I would not want to bother the developer once again, because I understand that this is a huge job, and as a musician I am not able to do anything like that at all. Once again I would like to express my gratitude! ) But I write something not about a problem much. So. If I enter the value 0.12 into the Specific Heat window, then when saving I will see 0. If 0,12 (as elsewhere, all numbers in the program are separated by a comma), then the program automatically changes 0,12 to 0.12, but while saving is carried out correctly. However, if you open the window again, and again click save without making changes, then again we get 0. I noticed that sometimes some numbers in the fields of the mash editor do not want to be saved (grain temperature, or temperature or pH of the Sparge water) I do not consider this a significant problem, since the save is guaranteed for the third time. Most of the time, saving happens immediately.

Which version of Brewtarget you are running? 3.0.5 and 3.0.6. The problem is present in both versions.

What operating system are you running? Windows 10, 64-bit. What language/country is your computer set up for? Hehe, I'm afraid that the attitude towards my personality may change, but I won't lie - I'm from Russia, so my system has Russian localization, but I use only Latin names, both in the username and in the program installation path. Brewtarget is also English, it's more convenient for me, the terms used in the program are well-established, and I don't feel the need to translate them. Are there any other places where you are seeing this problem with decimals, or is it only on the specific heat field? I encountered the problem with decimal places only in this field, the rest of the parameters are entered and saved correctly.

Forgive me if I bored you with such a large text, I guess. it could be shortened. I just wanted to explain in as much detail as possible.

matty0ung commented 1 year ago

Thanks, that's helpful.

It sounds like the field is using the correct decimal separator for input (comma for Russia, per https://en.wikipedia.org/wiki/Decimal_separator) but it then using the wrong separator for display (a dot), which then breaks the input when you save again. I'll have a look and see if I can work out why.

PS: No problem with you being Russian, or any other nationality for that matter! (It's what you say and do that counts, not where you live or what language you speak. I've met good and bad people in every country I ever went to.)

matty0ung commented 1 year ago

FYI, I more or less know what is causing this bug. Essentially the UI is treating the specific heat capacity as "string that might hold a number".

There is a short term fix, which is to add a new UI field type of "decimal number" (or similar), which is what I will probably do for now.

The more "correct" fix would be to incorporate specific heat capacity into our set of known physical quantities and their measurements, and give it its own field type (as we do for weights, volumes, temperatures, etc). This however requires a bit more thought. Usually we store everything internally in metric units (often SI ones). For specific heat we don't, we use the traditional unit of "calories per degree Celsius per gram" rather than the metric unit of "joules per degree Celsius per kilogram". Introducing a metric unit would probably break a bunch of things. So I might just implement the traditional unit and pretend the metric ones don't exist (because not even BeerJSON uses them). But I will seek a view from @mikfire as he usually has good input on these sorts of subjects.

mikfire commented 1 year ago

The thing that puzzles me here is that we rely on Qt to decide which decimal separator to use. Qt does this based on the system's locale. This suggests to me that either Qt is confused about your locale and so is choosing the wrong separator, or that we have an input field that is not invoking the proper methods to do the right thing. Given that this is only happening on one field, I am willing to bet the second problem. I will poke at it and see if I can spot a thing.

If you decide to take the "correct" fix, we store everything in SI. I cannot stress how much confusion could be caused by making an exception to that rule, and would recommend you do the hard work and fix it properly.

Jazzbeerman commented 1 year ago

If you decide to take the "correct" fix, we store everything in SI

Seems like a good idea! Yes, it's possible in the system locale. But SI is the world standard in many mathematical, physical, etc. processes. Even if you have to pay a little less convenience, in return for getting more stability, it would probably be very good.

P.S. For now, the workaround is to use a comma every time you need to save some gear changes. If it is not necessary to save, then the parameter is not reset during normal closing.

matty0ung commented 1 year ago

Thanks @mikfire. I think I found the immediate cause of the problem - viz the input being treated in one place as a string (so the bit where we should have asked Qt about the decimal separator was being skipped). It's a my-fault bug that stemmed from some of the work I did to make measurements more generic (because BeerJSON has so many new units of measurement, including for a whole range of things that simply aren't covered in BeerXML).

I think you are right about the metric/SI stuff. Having thought about it some more, I don't think it's enormous work to "do the right thing" internally, even though BeerJSON and BeerXML insist on calories over Joules. I'm going to do a short-term fix that gets things working without the need for a database update (but gets some of the new code ready in the background). Then will probably finish off when we bring in BeerJSON (as that will require DB updates anyway).

Jazzbeerman commented 1 year ago

Thank you! I checked. everything works without errors, now any saves do not reset the Specific heat value!