Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
312 stars 135 forks source link

Feedback on Mash Designer and Mash wizard. #204

Open pricelessbrewing opened 8 years ago

pricelessbrewing commented 8 years ago

Alright so finally trying this out in earnest. Having some issues understanding how the mash section in the bottom right is intended to be used. Incoming feedback from a new contributor and new user, hopefully this is helpful. Think you guys have a great piece of software here, and if it keeps improving could easily become very awesome! Sorry for the long post, probably a bit disorganized.

comments: Bottom of the window "mashs" is blank by default, didn't really understand what that was supposed to do until I clicked it. I would recommend making that single step by default. Seems like it's intended to pull up a previously saved mash profile?

Grain absorption as qt/lb is a little weird to me, I've never seen it referenced in those units before. Pretty much everyone I know uses gal/lb. Not a big issue, but a little strange none the less.

Editing the target temp directly in the mash window does not automatically adjust the infusion temp of the strike volume, which it probably should? Might already be a WIP, haven't read through all the other issues yet.

Mash window at the bottom does not display the mash thickness, only the mash designer, mash editor, mash wizard does. Inconsistent and a bit confusing for the new user. I realize not everyone thinks the same way, but don't most people choose their mash thickness and work their strike volume and strike temperature from that?

It seems to me the easiest way to set up a standby mash profile, change the target temp, then run the wizard again. Is that correct?

Mash Designer: For infusion it seems to me that the most logical slider wouldn't be the infusion volume, but the mash thickness, since that's usually what people are thinking of. Moreover, it's possible to exceed the recipe batch size, shouldn't there be an error if you exceed the preboil volume? Similarly, perhaps scale the "Total Collected Wort" percentage bar past 100%, so it's more easily understood when you exceed the calculated preboil volume? At the very least, it should say the correct number, even if it doesn't show it visually. Example: 6.982 gal total collected wort = 100% of 6.750 gal target preboil size. Terminology note: Why boil and not preboil? Could be confusing whether it means preboil or post boil. Trying to do a single infusion full volume mash, it's rather difficult as the "jumps" on the slide rules aren't continuous. Getting the exact right number is a bit difficult. Would be helpful if you could type in the number as well as move the slider.

Would be handy if their was an additional column to the mash window for the mash volume, run off volume, and if you end up doing so, the run off gravities for each step. Similar to what my mash calculator displays http://pricelessbrewing.github.io/BiabCalc If you do intend to implement more robust gravity estimator and batch sparge simulator, feel free to ask me any questions, the math is listed in my repo, don't mind the sloppy code and unclear variables.

mikfire commented 8 years ago

I like the idea of displaying the mash thickness. That has been an annoyance for me for a while. Now that I've figured out how to do it for the mash wizard, I will get to this. It likely won't be the 2.4 release.

I'm keeping it qt/lb. Sorry if it's weird. My site glass is incremented in quarts and liters. When I am measuring my strike water, I do not want to make the conversion from 4.5 gallons to 18 qts. Mostly because I start early in the morning, and yes I've screwed that math up before. YMMV, of course, and you are welcome to submit patches for it but only if those patches include an option to control it.

Editing the temp directly in the mash window should adjust ... something? I have looked at doing that many times, and I haven't seen the best way to do it. It sounds simple, but it gets complex fast. Raising the temperature of one step must necessarily raise the starting temperature of at least the step that follows, which will change its infusion temperature or maybe its volume (you can't change the temp of a decoction step easily), which could have implications further down the line. And whatever solution we come up with should try not to violate the principle of least astonishment, or at least be understood by the user when it happens. Nothing that cannot be solved, but something that simply hasn't annoyed me enough to fix.

I do not want to play with saved mashes. The entire idea needs to be rethought, re-engineered and reimplemented. I've some old ideas, but haven't been motivated enough to actually see about implementing. If you want to tackle it, I would suggest considering a "mash profile" that contains things like steps, final temps, percent preboil volume, etc, that can be displayed in a tree like an equipment profile. You drop it on a recipe, and all the maths are done to make that version of that profile adjusted to the recipe.

I would also really like to see creating a brewnote causing the mash to be adjusted. Since starting temperatures matter, and my starting temperature in July != my starting temperature in January, it would be cool to prompt the user and automagically adjust the mash.

pricelessbrewing commented 8 years ago

re: mash thickness units.

Understandable. I'll see if I can work out a patch to select that as an option once 2.4 is finalized.

Editing the temp directly in the mash window should adjust ... something?

Yeah, decoctions are weird like that. You'd have to treat them separately, and increase the decoction volume. But for infusions (strike / sparge) it would probably be best to keep the volume the same and adjust the temperature, as long as the temp doesn't exceed boiling. At which point I'd throw a red flag up and let the user know.

I do not want to play with saved mashes.

Honestly I'm not a big fan of the whole "profile" thing that. BUT If anything I would like to see initial mash temp disassociated from it, as that should be recipe specific, not process specific. Having the mash profile in charge of a recipe variable is just weird to me. The mash profiles should be process related, so it should include mash thickness, and whether you sparge or decoct and how many times you do so. Whether you mashout or not. After I know those things, it's fairly easy to calculate the rest out automatically.

Regardless, having the mash designer able to take input text (infusion volume, or mash thickness) instead of sliders would go a long way to being more accurate and user friendly.

Anything that's recipe specific, IMO should be displayed in the main window under the recipe section. Things like mash thickness, initial mash temp,

Thanks for the reply mikfire. I've been seeing quite a few comments on r/homebrewing the past few days with good things to say about brewtarget, helping to spread the word!

Daedalus12 commented 7 years ago

Been kicking around ideas on the mash table/designer/wizard trifecta. What about something like this for the mash table: image

The user sets the initial temperature and all target temperatures, and can specify for one (and only one) step a target thickness (or infusion temperature or amount). That drives the values for all the other steps. The limits on the user-controlled box can be determined so they can't set it to values that have impossible results for other steps.

Sparge steps are broken out into a lower section, and again the user can specify thickness for one sparge step that drives the others. The final sparge step always has a target of 100% collected wort.

I'm not sure I'm explaining this well, but might try and mock it up. It could essentially do away with the need for separate wizard and designer dialogs.

pricelessbrewing commented 7 years ago

Yes that would be suitable I think, and could completely do away with what I think is a clunky/strange wizard/designer dialog process. I would replace Amount with Volume, and I like the Tun Fullness column! I think that would be MUCH more user friendly, and faster to use as well! I don't understand what the "Manual" column is referring to.

Once the 2.4 release is finalized, I'll look into the code some more and see if I can implement my efficiency prediction formulas.

Daedalus12 commented 7 years ago

I agree it should be "Volume" instead of "Amount".

"Manual" is a bad name for that column; "Target" would be better. I'm thinking of it as a drop-down menu where you can select from the following options:

To satisfy the requirement that there only be one driving step:

eltomek commented 7 years ago

@Daedalus12 I fully support mash wizard/table rework but I think that many controlling options will make it complicated no less than it is today.

Is there a real life scenario for having mash step driven by a volume of collected wort or its thickness? The cases that I always used are:

What I dislike in the current implementation in Mash wizard is mainly that some text boxes (like Infuse Amount and Infuse Temp for Infusion type of step) are editable although they are expected to be calculated (for these particular ones when whatever you dial in there, after applying Mash Wiz it will bring the Infuse Temp to 100C (boiling water) and the Infuse Amount accordingly), the same applies to the mashStepTableWidget which should look like you've proposed (specific fields disabled).

image

Also, every time you make any change in the target temps you need to call Mash wiz again, verify if the Mash thickness is what you've dialed in before (it sometimes changes) and it will re-calc appropriately. I think we might improve it a bit by re-calculating upon onChange, these are not very complex calculations.

As for Mash Des it should not be asking for initial tun temperature, as it has this info in Mash entry (Edit mash button). This is a predefined set of parameters and could be enhanced with remaining useful (and batch-specific) info like i.a sparge method, number of barches or mash thickness to name just a few and serve this information for Mash Designer and Mash Wizard (do we need both by the way?) so they do not ask user every time. By the way, pressing Mash Des overwrites the mash steps (that's an issue).

EDIT: I see your point on mash thickness driving mash step, this is for the initial (strike water temp) step if I understand correctly.

mikfire commented 7 years ago

I've tried for years to redesign this stuff, and have not even had the beginnings of an idea. I would suggest a few things to keep in mind while you are thinking about redesigning this part.

My experience is that the mash wizard/designer/whatever is the hardest part for a new user. What ever you come up with, try really hard to make it easy for the beginning brewer or at least the new brewtarget user.

I'm a lazy brewer. I do single step infusion mashes. I usually put in my desired mash temp, and then use the wizard to calculate the rest. What ever you come up with, it has to allow me to continue to be at least this lazy.

pricelessbrewing commented 7 years ago

Note entirely sure how to quote on here, so forgive me if my formatting is off.

"Manual" is a bad name for that column; "Target" would be better. I'm thinking of it as a drop-down menu where you can select from the following options:

Thickness (makes this the driving step, user can input value in Thickness column)
Volume (makes this the driving step, user can input value in Volume column)
Infusion Temp (makes this the driving step, user can input value in Infusion Temp column)
Tun Fullness (makes this the driving step, user can input value in Tun Fullness column)
Collected Wort (makes this the driving step, user can input value in Collected Wort column)
Fixed (user can input values in Volume and Infusion Temp column, Target Temp becomes a calculated value and is grayed out. This is how legacy mash steps would import.)
Auto (makes this step driven by other steps; no input values other than Target Temp.)

To satisfy the requirement that there only be one driving step:

When the user tries to set all steps to auto, you get an error message.
When the user tries to set multiple steps to NOT auto, the other steps switch to auto.

It seems to me that all of these can actually just go to the background and not be displayed. If a user changes a variable, collected wort, infusion temp, whatever, it should automatically change the type of "Target" it is.

Thickness (makes this the driving step, user can input value in Thickness column)

A change in thickness drives changes in infusion volume, and therefore collected wort, and volume. Example: Most mashtun brewers determine their initial sacc rest infusion volume via mash thickness. The main exception being BIAB / no sparge / full volume mashers, which determine it via "Collected wort" = Preboil volume.

Volume (makes this the driving step, user can input value in Volume column)

A change in volume* drives calculated changes in mash thickness, mashtun fullness, collected wort** and infusion temp.

*infusion volume, right? Not mash volume? Can you add that as a column, where applicable it shows mash volume as well as mashtun fullness?

**Collected wort should probably be called "Run off volume" as that's the standard nomenclature, although collected wort is much more obvious to the new brewer.

mash volume = infusion volume (Gallons) + ( Grain Bill (lb) * 0.08 (grain displacement gallons/lb) )

Volume (makes this the driving step, user can input value in Volume column)

This will be a rare situation where a brewer determines their mash thickness via a fixed strike volume measurement, but some do use the same strike volume for all of their brews. Drives calculated changes to infusion temp, mashtun fullness, collected wort, as well as the respective "children" mash steps.

Infusion Temp (makes this the driving step, user can input value in Infusion Temp column)

I see no situation where this would ever be used.

Tun Fullness (makes this the driving step, user can input value in Tun Fullness column)

Maxi-BIAB'ers, or people that mash to a fixed volume (sometimes i have to do this for big brews) may use this. However it can be adjusted by manipulating mashtun thickness OR infusion volume. I would be fine with this being a non-input cell.

Collected Wort (makes this the driving step, user can input value in Collected Wort column)

Again, this is a calculated variable, I see no reason to allow this as an input parameter.

Fixed (user can input values in Volume and Infusion Temp column, Target Temp becomes a calculated value and is grayed out. This is how legacy mash steps would import.)
Auto (makes this step driven by other steps; no input values other than Target Temp.)

I also think the default mash step should be single infusion starch conversion rest (152 or 154F)

  1. initial temperature
  2. Conversion rest, 60 minutes. Mash thickness 1.5 Qt/Lb.
  3. Sparge Step 1: Calculated infusion volume so that collected wort = 100%.

This will be applicable to the majority of users, and those that desired multi-step infusions, or multiple sparge steps should be knowledgeable enough to figure out how to add additional steps (which should be added to the bottom). Those that do not sparge, can easily change this by deleting the batch sparge step, which should recalculate the starch conversion rest step so that the mash thickness is corresponding to 100% collected wort. If not, the total collected wort should be RED flagged to notify the user.

I think that would be clean, and easy to use, and is fairly similar to what I use for my mash calculator. I can help with any math questions :)

This will likely require a bunch of if-thens, as that's how I had to do it in order to allow users to input both mash thickness, and infusion volume. In case it saves time, here's what I did for this. May not be the most efficient, but it works.

Previous sparge is what the previous volume value would be, and mash thickness is the previous mash thickness value. MashThick is the new calculated mash thickness, based on current user inputs, and VolSparge2 is what infusion volume would be required for a single sparge process to give 100% collected wort. If mash thickness is changed, it changes the sparge volume automatically to compensate, and vice versa.

//Make a new sparge input set mashThickness to zero
  if (PreviousSparge !== VolSparge)
    { if (VolSparge > 0) {
     MashThickness = 0;
  byId('MashThickness').value = MashThickness;
  PreviousThickness = MashThickness;
  }
    }

  //Make a new MashThickness input set SpargeVolume to zero
  if (PreviousThickness !== MashThickness)
    {
    if (MashThickness > 0) {
     VolSparge = 0;
  byId('VolSparge').value = VolSparge;
    }
    }

  MashThick = (WaterTot - VolSparge) * 4 / GBill;
  if (MashThickness == 0) {
    VolSparge2 = 0;
  } else {
    VolSparge2 = WaterTot - (GBill * MashThickness / 4);
    MashThick = MashThickness;
    VolSparge = VolSparge2;
  }
eltomek commented 7 years ago

This is really good discussion and promising development direction of this part of BrewTarget. How about if we prototype the table and dialogs in the online spreadsheet before actual coding in BT? That might be easier to cooperate on the solution which looks to have dozens of needs and scenarios to be addressed.

I am definitely for having some predefined mash profiles and enable user to save his own, and at start user could be presented with the simplest one (Single-step infusion mash) with the possibility to select some other (like it is today).

I set up an initial spreadsheet for that purpose at https://docs.google.com/spreadsheets/d/1lVbOliVABgOvHYmss-SwL7K-OYtcgLBUDp7vq20FQBo/edit?usp=sharing with some predefined mash profiles proposal.

Also, for me the mash thickness (the value that is relevant just at the 1st step) is more the mash property rather than step property and I'd see it more in the Mash Editor rather than as a column in the table, but that's just a suggesion.

The mash tun parameters like tun mass or its specific heat shall be part of Equipment property (and thus present on Equipment Editor), the only info that we need in the mash editor is it's initial temperature. We don't even need it listed in the Mash Editor as it's attached to the recipe.

pricelessbrewing commented 7 years ago

I'm not sure if this type of multiple simultaneous inputs can be done in a spreadsheet.

I can see if I can create a prototype version online though using js, jQuery, and html when I get home to demonstrate.

On Feb 4, 2017 5:19 AM, "Tomek Lorek" notifications@github.com wrote:

This is really good discussion and promising development direction of this part of BrewTarget. How about if we prototype the table and dialogs in the online spreadsheet before actual coding in BT? That might be easier to cooperate on the solution which looks to have dozens of need that need to be addressed. I set up an initial spreadsheet for that purpose at https://docs.google.com/spreadsheets/d/1lVbOliVABgOvHYmss-SwL7K- OYtcgLBUDp7vq20FQBo/edit?usp=sharing

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Brewtarget/brewtarget/issues/204#issuecomment-277433725, or mute the thread https://github.com/notifications/unsubscribe-auth/AIZ39YhO_MgCxxAOCY1RJvWV-ewCI66Aks5rZFDEgaJpZM4HoLG0 .

eltomek commented 7 years ago

@pricelessbrewing it will handle it, if not from the spreadsheet level you can enhance it easily with JavaScript (https://developers.google.com/apps-script/guides/sheets). Though I think it will be more than enough, but if you have another option to allow collaborative work feel free to share.

pricelessbrewing commented 7 years ago

Neat! I didn't know Google sheets could do that!

On Feb 4, 2017 10:04 AM, "Tomek Lorek" notifications@github.com wrote:

@pricelessbrewing https://github.com/pricelessbrewing it will handle it, if not from the spreadsheet level you can enhance it easily with JavaScript (https://developers.google.com/apps-script/guides/sheets). Though I think it will be more than enough.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Brewtarget/brewtarget/issues/204#issuecomment-277451550, or mute the thread https://github.com/notifications/unsubscribe-auth/AIZ39fRZsuDHV1kmv85pKOzZhVZAW4FZks5rZJOKgaJpZM4HoLG0 .

Daedalus12 commented 7 years ago

@mikfire, I agree it's a tough problem to make the mash features easy-to-use while simultaneously supporting all the different use cases. I do mostly BIAB single-step full-volume mashes these days, definitely interested in keeping things simple for those most-common use cases!

Have been playing with it more and thinking on how to avoid making it unfamiliar for existing users. One way would be to have double-clicking a mash step launch a dialog very similar to the current Mash Designer. Rather than directly editing cells in the table, the user would drag around sliders in the dialog while seeing what affect it would have on the other mash steps in the table. The limits of the sliders are defined by calculations using the other mash steps (to avoid infusion temperatures above boiling, etc.).

The existing Mash Designer dialog is pretty great, it gives the user a good sense of the interconnectedness of the parameters. What I'm proposing would basically be a change to use that dialog as an editor for a single step, rather than for all steps.

pricelessbrewing commented 6 years ago

Finally got 2.4.0 to build!

More on Mash Designer. It initially started as just a feature request and a bug, but kept growing so figured this might be a better place rather than opening new issues?

I love that it shows the mash thickness on the right in the designer, but as soon as you touch the sliders the labels go away for thickness and volume, can we make those persistent?

I think it would be really useful though to allow the mash thickness to be allow for input values as well, and/or to change the output infusion volumes/temps to an input cell that is centered beneath the sliders. Would increase usability, and make the workflow a bit quicker if you say wanted to perform an infusion at exactly 5.000 gallons for instance. For the recipe I have open right now, the sliders do not allow for that, it goes from 4.972 to 5.007. Obviously there's little difference between 5.007 from 5.000, but if we're going to show precision to three decimal places than we should allow for steps at 0.001 right?

Additionally, I tried to use the arrow keys on my keyboard to see if there was any benefit for more refined control of the sliders. While it DOES move the sliders left and right, no calculations are performed. So you can move the slider all the way from the right to the left, without it affected anything.

When creating a step where the Total collected wort volume is greater than 100%, the bar remains full, and the percentage remains at 100% despite exceeding the value. Example: I can set a target temp that is impossible given the total collected wort maximum, like 172F, which for this particular recipe and a previous mash step, would give a total collected wort volume of 10.157 gal out of a max of 6.674 gallons. Still shows 100%. Perhaps have it refill with a red bar for any value exceeding 100%, and allow for a value of 152%?

When I made an infusion, then a batch sparge. The infusion temperature of the batch sparge is still bugged by the negative volume/temp issue. Currently it's -202.764F, while the slider shows a minimum of 157.707F to max of 212F. Once the infusion temp for the batch sparge is set to something realistic (150-200F), then the infusion volume becomes negative (-5.175F currently). Similarly for fly sparging. Looks like #94 except for sparge step types.

Lastly. Having total collected wort and Tun Fullness go from 0.000 mL to X.XXX gal is a bit weird. I'm sure it has to do with how small values are supported in the unit conversion system and automatically changing to support precision, but shouldn't it remain the same unit type, or at the very least it should stay within the same unit system (so fl oz or tsp maybe for US, not mL)?

kapinga commented 6 years ago

I very much agree that some additional improvements should be made to the MashDesigner. In particular, users should have the option to manually enter any of the values and have everything else recalculate. It's on my todo list, but I'm not able to dedicate very much time to this project (and a novice in contributing to open source projects), so I'm happy if someone else gets it done instead.

I also agree on the units issue, although that complaint is program-wide (and probably deserves its own issue). I'm not sure why your build is switching between gallons and mL though; mine current build (develop + PR #382 ) scales from gallons down through to teaspoons. Note: I have my volume units (in options) set to US traditional (not, e.g., British Imperial).

Some of your points are addressed already I think in existing PRs. In particular

pricelessbrewing commented 6 years ago

Thanks @kapinga I just built from the main (develop) branch yesterday though. Is there a more up to date develop branch that I'm not aware of? I see PR #382 is still waiting to be merged, so it's possible that it is fixed, but still awaiting review.

Units

Mine is set to US traditional as well. It appears to be fixed now, so it appears closing and re-opening brewtarget fixes this, so it should be labelled as a bug. Not a huge priority, but still.

pricelessbrewing commented 6 years ago

So I never got around to learning how the google sheets thing works, but I started working on this as a side project, not sure if I'll end up completing it or not.

What I think of as "cascading" mash steps. If you change a value, it calculates the rest of that row, then cascades down. All maximums and minimums are based on the recipe and equipment profile. So you can't enter a mash infusion of 20 gallons if you only need 7.5 gallons until you edit your equipment profile accordingly.

https://www.codeply.com/go/igJQVHTR6s

Please ignore the mess of code, was still playing around and experimenting with some things.

Basically break all mash rows into steps, where the type of step is a dropdown (infusion, mash, heat, runoff / drain mashtun, sparge, decoction, sour). Then each step has an appropriate value for

So starting from the default values

If you want to do a single infusion, you can either change the strike volume, mash step 1 thickness, or step two (infusion) volume, or mash step 2 thickness. It will automatically adjust accordingly. If a value is zero, it first ignores it, and proceeds downward, if that is impossible it will change an adjacent value accordingly or reset the cell you input accordingly.

The default is set for a no sparge BIAB style brew day. to change to two stage step mash, you can change the thickness to say 1.75 qt/lb. It will calculate the infusion volume necessary (2.46 gallons).

If you then want to add a sparge, you can lower the thickness from 1.75 qt/lb to 1.25 qt/lb and the second stage (now infusion volume of 3.96 gallons), to 2 gallons, and you'll end up with

Mash in at 154F, infusion of 2 gallons of water, sparge with 1.96 gallons.

From there we would only need buttons for Add step, remove step, maybe make the steps draggable and/or up/down arrows.

Thoughts? @kapinga @Daedalus12 @eltomek @mikfire @dpettersson ?