search

svalinn / DAGMC

Direct Accelerated Geometry Monte Carlo Toolkit
https://svalinn.github.io/DAGMC
Other
96 stars 63 forks source link

Adding console output methods to some classes #872

Closed pshriwise closed 1 year ago

pshriwise commented 1 year ago

This is a step toward improved console logging in some of our classes. As described in #856, DAGMC and other classes in MOAB always write some output, which can conflict with output in downstream applications and cause confusion.

message, warning, and err_msg methods have been added to the dagmcMetaData and DagMC classes. Warnings and errors will always be written to screen. The message method allows a verbosity level to be associated with the message. The message will either be written to screen or not depending on the verbosity level set on the class instance. I've specified two levels of verbosity for now, but I'm open to a more fine grain set of verbosity levels down the line.

There's some duplicated code here, which could be improved. I'll look into abstracting these capabilieis into an object that both of the affected classes can inherit from perhaps.

Another thing I don't love about this is that a std::stringstream object needs to be created for messages where conversion of non-char/string types occurs via the streaming operator. There are other libraries like fmt that handle this more gracefully, but I'll leave that for an improvement in the future.

gonuke commented 1 year ago

Did you mean this to be extended from #855 (which we should get back to)?

pshriwise commented 1 year ago

Did you mean this to be extended from #855 (which we should get back to)?

Ugh no. Accidentally combined some stashed changes I think. Fixing...

gonuke commented 1 year ago

I've merged #855, so maybe it will be easy to just rebase on that?

gonuke commented 1 year ago

If you rebase we can probably more forward on this @pshriwise

pshriwise commented 1 year ago

Thanks for the review @gonuke! Not sure why the Housekeeping/Linux tests are failing at this point. Something about git permissions in the CI image. Made an attempt at addressing it, but no luck yet. I'll try to revisit it later if I have time.

gonuke commented 1 year ago

The failing the linux builds is something annoying that I was sure we had resolved many months ago. Not sure why it's coming up again.

gonuke commented 1 year ago

I made a quick scan for logging libraries and didn't find anything compelling - all a little heavier weight than I'd like.

Do you think we could write a header-only small class that would do this and include it in the relevant places?

pshriwise commented 1 year ago

We certainly could! I don't have the cycles to do so at the moment though

gonuke commented 1 year ago

We certainly could! I don't have the cycles to do so at the moment though

Let me see how I might adapt what you have done in that way

gonuke commented 1 year ago

I think this was superseded by #876

pshriwise commented 1 year ago

Closing in favor of #876. @ahnaf-tahmid-chowdhury, if the changes you've suggested above apply in that PR as well would you mind adding them there also?