-
Notifications
You must be signed in to change notification settings - Fork 187
Develop an API for a STDLIB logging system #227
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
Comments
Thank you @wclodius2. Note #193.
I think so.
Yes. There may not be as much excitement or intense discussion on the topic as you'd like, but we should still go forward with it and iterate as issues come up.
I think stdout should be default.
Yes.
As I understand it, it seems to me the choice is between 1 and as many as you want, depending on the implementation. Multiple loggers in the same application seems like it'd be useful for multi-component programs.
This seems like a matter of style. Personally I think having separate procedures Let's discuss it on our call next week: https://fortran-lang.discourse.group/t/fortran-monthly-call-august-2020 |
The tentative API I have developed is described below. The module STDLIB_LOGGERIntroductionThis module defines procedures to be used for reporting by the Fortran The logger has the options to:
The STDLIB_LOGGER constantsThe module defines seven distinct named integer constants for The STDLIB_LOGGER module proceduresOverview of the proceduresThe module defines thirteen public procedures: one function and twelve
There are three subroutines for manipulating
There are seven subroutines for sending messages to the
There is one subroutine for configuring the logger:
There is one subroutine for reporting the details of the logging
|
Thank you, @wclodius2. Can you make it to the call today? https://fortran-lang.discourse.group/t/fortran-monthly-call-august-2020/260/2 It'd be good to discuss it live and get a temperature on what people think about it. |
I will try to make it, but it will be my first zoom.
… On Aug 20, 2020, at 8:34 AM, Milan Curcic ***@***.***> wrote:
Thank you, @wclodius2 <https://github.com/wclodius2>. Can you make it to the call today? https://fortran-lang.discourse.group/t/fortran-monthly-call-august-2020/260/2 <https://fortran-lang.discourse.group/t/fortran-monthly-call-august-2020/260/2>
It'd be good to discuss it live and get a temperature on what people think about it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#227 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APTQDOUK2QYUIXG2IYODA5TSBUX7TANCNFSM4PYCWD2Q>.
|
Given that the response seemed positive at the call I will now proceed. Next should be a detailed outline of the proposed API, followed by an implementation of any changes in the API, then testing, then a checkout of the STDLIB source, and then a PR for the implementation and tests. What follows is a detailed summary of the procedures in a form that parallels the format used in the standard. Possible sources of controversy are:
My response to the above are
Specification of the ProceduresADD_LOG_FILE( FILENAME, UNIT [, ACTION, POSITION, STATUS, STAT ] )Description. Sets Class. Subroutine Arguments. FILENAME shall be a scalar default UNIT shall be a scalar default ACTION (optional) shall be a scalar default POSITION (optional) shall be a scalar default STATUS (optional) shall be a scalar default STAT (optional) shall be a scalar default ADD_LOG_UNIT( UNIT [, STAT ] )Description Adds Class. Subroutine. Arguments. UNIT shall be a scalar default STAT (optional) shaa be a scalar default
ASSERT( TEST, MESSAGE [, MODULE, PROCEDURE ] )Description. Checks the value of Behavior. If Class. Subroutine. Arguments. TEST shall be a scalar default MESSAGE shall be a scalar default MODULE (optional) shall be a scalar default PROCEDURE (optional) shall be a scalar default CONFIGURE_LOGGING( [ ADD_LINE, INDENT, MAX_WIDTH, TIME_STAMP ] )Description. Configures the logging process. Class. Subroutine. Arguments. ADD_LINE (optional) shall be a scalar default INDENT (optional) shall be a scalar default MAX_WIDTH (optional) shall be a scalar default TIME_STAMP (optional) shall be a scalar default LOG_ERROR( MESSAGE [, MODULE, PROCEDURE, STAT, ERRMSG ] )Description. Writes the string Behavior. If time stamps are active, a time stamp is written Class. Subroutine. MESSAGE shall be a scalar default MODULE (optional) shall be a scalar default PROCEDURE (optional) shall be a scalar default STAT (optional) shall be a scalar default ERRMSG (optional) shall be a scalar default LOG_INFORMATION( MESSAGE [, MODULE, PROCEDURE ] )Description. Writes the string Behavior. If time stamps are active, a time stamp is written Class. Subroutine. MESSAGE shall be a scalar default MODULE (optional) shall be a scalar default PROCEDURE (optional) shall be a scalar default LOG_IO_ERROR( MESSAGE [, MODULE, PROCEDURE, IOSTAT, IOMSG ] )Description. Writes the string Behavior. If time stamps are active, a time stamp is written Class. Subroutine. MESSAGE shall be a scalar default MODULE (optional) shall be a scalar default PROCEDURE (optional) shall be a scalar default IOSTAT (optional) shall be a scalar default IOMSG (optional) shall be a scalar default LOG_MESSAGE( MESSAGE [, MODULE, PROCEDURE ] )Description. Writes the string Behavior. If time stamps are active, a time stamp is written Class. Subroutine. MESSAGE shall be a scalar default MODULE (optional) shall be a scalar default PROCEDURE (optional) shall be a scalar default LOG_TEXT_ERROR( LINE, COLUMN, DESCRIP [, FILENAME, LINE_NUMBER, STAT ] )Description. Behavior. If time stamps are active first a time stamp is Class. Subroutine. Arguments. LINE shall be a scalar default COLUMN shall be a scalar default DESCRIP shall be a scalar default FILENAME (optional) shall be a scalar default LINE_NUMBER (optional) shall be a scalar default STAT (optional) shall be a scalar default LOG_UNITS_ASIGNED()Description. Returns Class. Function. Result Character. The result is a scalar default Result Value. The result is LOG_WARNING( MESSAGE [, MODULE, PROCEDURE ] )Description. Writes the string Behavior. If time stamps are active, a time stamp is written Class. Subroutine. MESSAGE shall be a scalar default MODULE (optional) shall be a scalar default PROCEDURE (optional) shall be a scalar default LOGGING_CONFIGURATION( [ ADD_LINE, INDENT, MAX_WIDTH, TIME_STAMP, LOG_UNITS ] )Description. Reports the logging configuration. Class. Subroutine. Arguments. ADD_LINE (optional) shall be a scalar default INDENT (optional) shall be a scalar default MAX_WIDTH (optional) shall be a scalar default TIME_STAMP (optional) shall be a scalar default LOG_UNITS (optional) shall be a rank one allocatable array REMOVE_LOG_UNIT( UNIT [, CLOSE_UNIT ] )Description. Remove Class. Subroutine. Arguments UNIT shall be a scalar default CLOSE_UNIT (optional) shall be a scalar default |
Hi @wclodius2, thank you for the detailed spec. At this time I'll only comment on the proposal of I think you can go ahead with the PR and I will assist you with the edits and testing. The PR should also include the spec following an existing format (see examples here). It looks like you already have all the information that goes into the spec, it just needs to be re-formatted. Also, please follow the Style guide. |
@milancurcic sorry for the delay. I had ben working on the BITSETS code, and wanted it to successfully compile before I put put it aside. I have decided to convert the logger into a derived type, The existing specs only cover procedures, and I will have to cover more than that so there will be new ground: the derived type, module defined constants, and a module variable, |
@milancurcic I have the stdlib_logger module and the test_stdlib_logger test code successfully compiling and running. The test code output is a bit messy, but so is the API it is testing so I don't see how to make it much cleaner. I think I am testing everything except the failures that result in an error stop. If testing of those is desired I will probably need a separate test code for each error stop. I also have a markdown document documenting the API. Before I check in the code and document I want to test it out using |
Never mind the integrating of |
I have finished testing the code with cmake. Now how do I submit a PR for stdlib? I have tried to push the revised code but I get the error message The internet suggests that the most common cause of the 403 error is a bad password, but changing the password to something known to be bad now gives a different message : Do you know how to fix the 403 error? FWIW I have an older fork of stdlib here, but I can't seem to merge recent changes in the source directories. Should I upload the revised files there anyway and do a PR from there? |
Hi William, yes, you need to create a new branch on your fork (wclodius2/stdlib), then open a new Pull Request from the fortran-lang/stdlib page (there will be a yellow prompt near the top). As an aside, 403 means Forbidden, which indicates correct password but lack of permission to write to fortran-lang/stdlib. In your second case, the error is 401 Unauthorized. That's why you get a different answer in those two attempts. |
Hi Milan. I have done a PR from my fork. The main files to concentrate on are the doc/specs/stdlib_logger.md, src/stdlib_logger.f90, and src/tests/logger/test_stdlib_logger.f90. I have also provided mods of the pertinent CMakeLists.txt and Makefile.manual files to be compatible with my fork, but I didn't know if you wanted those to be compatible with my fork, or with the current main branch, or with whatever branch you work with. Fortunately, they are easy to modify to be compatible with what you want. I am fairly happy with the stdlib_logger.md and stdlib_logger.f90 files. I am less happy with the test_stdlib_logger.f90 file, but it does what needs to be done. |
Besides the log_io_error method, may I suggest adding log_alloc_error and log_dealloc_error methods? ALLOCATE and DEALLOCATE also have optional stat= error returns. We included specific methods for alloc, and dealloc, and also a NETCDF variant, in the ESMF logger class: Probably should have done a MPI variant as well. But in ESMF, the vast majority of MPI calls are in the C++ code. I should note that we wrote several of the methods as 'found' functions with logical results - e.g., ESMF_LogFoundAllocError, as they could then be combined with the inevitable IF statement which follows an error checked statement. Made the error checking code a little cleaner and more compact. |
How is having |
In the case of ESMF, we issued specific memory allocation/deallocation messages, and also specific error 'return codes' to pass up the call chain. |
With the NETCDF variant, we called the NETCDF error message routines (e.g., NF90_STRERROR) and included their messages in our log as well. |
Should also note that in ESMF, we wanted to distinguish between our library 'return codes' (rc=) and Fortran-specific iostat/stat status returns. So the different entry points into the Log class reflected that as well. |
This issue was addressed in |
It is common for a library of code to have the equivalent of one or more error logging modules. SLATEC has its XERROR codes, Futility has its ExceptionHandler.f90 and FileTypeLog.f90, and FLIBS has its reporting, m_logger, m_multilog, and m_exception modules. I propose that STDLIB have its own logger module that I propose naming
STDLIB_LOGGER
. I propose that the logger have options to:OUTPUT_UNIT
orERROR_UNIT
;yyyy-mm-dd hh:mm:ss.sss
;STAT
andERRMSG
of the statement that prompted the log message;IOSTAT
andIOMSG
of theI/O
statement that prompted the log message;'INFORMATION: '
,'WARNING: '
, or'ERROR: '
;The maximum number of logical units to be supported at one time is a minimum of two: one of either
OUTPUT_UNIT
orERROR_UNIT
and an additional formatted file, but by using an array of logical units it should be possible to support more, such as the five supported by SLATEC's XERROR. While the initial implementation will use Fortran's standard file interface through logical units, I hope to hide enough of the details that wrappers to the logical units can be deployed if desired. The above proposal prompts the following questions:OUTPUT_UNIT
orERROR_UNIT
?OUTPUT_UNIT
andERROR_UNIT
at the same time?LOG_ERROR
,LOG_WARNING
, andLOG_INFO
, or by a flag?The text was updated successfully, but these errors were encountered: