search

reilleya / openMotor

An open-source internal ballistics simulator for rocket motor experimenters
GNU General Public License v3.0
398 stars 78 forks source link

Flow Separation Warning #205

Closed AstroChuck closed 1 year ago

AstroChuck commented 1 year ago

If the simulation shows an exit pressure which is below the Summerfield Criteria for flow separation, a warning will be generated.

AstroChuck commented 1 year ago

Both changes have been implemented. I also added some import logic so older files will have the correct default separation ratio. I also added a fix where the start and end data points would trigger the warning, so the warning now only triggers if the condition persists. I also recalled you were trying to keep the commits clean, so I squashed this commit with the previous one so the PR only has a single commit.

petterreinholdtsen commented 1 year ago

The test build of this patch fail. Perhaps something to look at?

AstroChuck commented 1 year ago

All new requested changes have been implemented.

reilleya commented 1 year ago

I finally got around to playing with this and really like it! Last issue is that the warning text is too long, at least on my laptop: image I know what it says so it isn't a problem for me, but it seems like we should either find a more concise message, make this window resizable, or at least make it wider. I'd be happy to merge it with that changed!

benrussell11 commented 1 year ago

Hi Andrew,

Is the latest available from “Stage”. I’d like to give it a try.

Ben

From: Andrew Reilley @.> Sent: Thursday, May 18, 2023 11:33 PM To: reilleya/openMotor @.> Cc: Subscribed @.***> Subject: Re: [reilleya/openMotor] Flow Separation Warning (PR #205)

I finally got around to playing with this and really like it! Last issue is that the warning text is too long, at least on my laptop: https://user-images.githubusercontent.com/9787344/238164845-83af2ed5-2fc3-49ca-baec-d98acaaa1592.png I know what it says so it isn't a problem for me, but it seems like we should either find a more concise message, make this window resizable, or at least make it wider. I'd be happy to merge it with that changed!

— Reply to this email directly, view it on GitHub https://github.com/reilleya/openMotor/pull/205#issuecomment-1553949602 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AL4HO6G5YZA25EX2X7SD2OTXG3SYTANCNFSM6AAAAAATULZMFM . You are receiving this because you are subscribed to this thread. https://github.com/notifications/beacon/AL4HO6FU4O3VSNXVCCR4WETXG3SYTA5CNFSM6AAAAAATULZMFOWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTS4T5R2E.gif Message ID: @. @.> >

reilleya commented 1 year ago

Not quite! It will be once this is merged, which will happen as soon as we touch up the text in the warning.

AstroChuck commented 1 year ago

I've made the requested change, the warning text is now approximately the same length as the other warnings. A longer term future improvement seems to be warranted to improve the warning message GUI to support more verbose and informative messages, but I'm going to put a pin in that for now.

benrussell11 commented 1 year ago

Thanks Andrew. Will give it a try.

Ben

From: Andrew Reilley @.> Sent: Monday, May 29, 2023 9:41 PM To: reilleya/openMotor @.> Cc: benrussell11 @.>; Comment @.> Subject: Re: [reilleya/openMotor] Flow Separation Warning (PR #205)

Merged #205 https://github.com/reilleya/openMotor/pull/205 into staging.

— Reply to this email directly, view it on GitHub https://github.com/reilleya/openMotor/pull/205#event-9372840272 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AL4HO6DPOHKBDVL4E3NX5U3XIVF3ZANCNFSM6AAAAAATULZMFM . You are receiving this because you commented. https://github.com/notifications/beacon/AL4HO6G2THJIL5NRDKC5Q5TXIVF3ZA5CNFSM6AAAAAATULZMFOWGG33NNVSW45C7OR4XAZNWJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XKUY3PNVWWK3TUL5UWJTYAAAAAELVKGFIA.gif Message ID: @. @.> >