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

Prevent spamming output with 'Not realizable' #142

Merged
merged 15 commits into from Jan 24, 2020
Merged

Prevent spamming output with 'Not realizable' #142

merged 15 commits into from Jan 24, 2020

Conversation

joeweaver
Copy link
Contributor

N.B. I'm not entirely sure of the development flow for this project. I've created a fork, tested my code in the development branch, and am creating a pull request. I'm also opening an issue in the parent repository to associate with this PR. I've tried to make an informative commit message, but it looks like you might have your own format. Happy to adjust things.

When the ODE solver encountered an unrealizable situation, it sent the string 'Not realizable' to the terminal each time the local time step was adjusted. This spammed the terminal/logs and was somewhat uninformative to new users and has been accidentally considered a failed run, rather than part of the ODE solution process. The string is now only output the first time the local time step is adjusted and has been reworded to hopefully reduce confusion.

AlbertoPa and others added 15 commits November 14, 2019 01:32
When the ODE solver encountered an unrealizable situation, it sent the string 'Not realizeable' to the terminal each time the local time step was adjusted.  This spammed the terminal/logs and was somewhat uniformative to new users and has been accidnetally considered a failed run, rather than part of the ODE solution process.  The string is now only output the first time the local time step is adjusted and has been reworded to hopefully reduce confusion.
@joeweaver
Copy link
Contributor Author

Something is up with the files changed in this PR - I've only edited realizableOdeSolver.C & realizableOdeSolver.H. I may have goofed something in forking or creating the PR, since I'm not terribly experienced in multiuser, multibranch git workflows.

@AlbertoPa
Copy link
Member

Thank you for your PR. I agree the message was not particularly informative. The workflow we have is quite simple and what you have followed is perfectly fine.

I think the problem with the additional commits is on my side. It looks like our development branch on GitHub is missing some of those commits, but I have them locally. I am going to look into that and then merge your PR.

Copy link
Member

@AlbertoPa AlbertoPa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants