-
Notifications
You must be signed in to change notification settings - Fork 1
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
Create log file vesuvio #153
Open
GuiMacielPereira
wants to merge
13
commits into
main
Choose a base branch
from
create-log-file-vesuvio
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GuiMacielPereira
force-pushed
the
create-log-file-vesuvio
branch
from
January 7, 2025 14:46
eccffe0
to
f04ec1d
Compare
Logging settings are added during config command. Current downside is that if logging settings are set to mantid properties when mantid is open, mantid needs to be closed and opened again for changes to take place.
Added function that takes in existing mantid log file and produces a clean version with only the important information.
Replaced print statements in fitting with logger.notice(). Added these statements to log file.
Changed the name of the log file to account for the fact that one of the routines is joint, so hte name cannot depend on the spectra being fitted (well it could, but that would be unecessary and more complex). I also noticed that no information about the initial parameters was being outputed so I created a new function for printing tables and used that to show the tables for the initial parameters.
Convert to integers where possible so that priting of zeros is avoided. Changed names of columns of fit parameters because they were taking too much space when the table got print, which made it very difficult to read. Now table occupies much less space and is much clearer to read.
Previous notices were not very clear and made the code more confusing, replaced them with printing tables that are already bring created anyway. Log output is much easier to follow this way
GuiMacielPereira
force-pushed
the
create-log-file-vesuvio
branch
from
January 7, 2025 14:54
f04ec1d
to
73bc753
Compare
GuiMacielPereira
added
enhancement
New feature or request
maintenance
vesuvio-analysis
labels
Jan 7, 2025
7 tasks
Unit test for making a summarised log file from mantid log file
I am getting a permission error when I click on Run:
|
The log file cannot be cleaned when on Windows, because due to file locks, cannot open file when it is being used by Mantid to write to it. The best solution for now is to clean the file when on Unix system. On windows the log file will still be produced, but will contain the logs of all times the routines were run.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of work:
Mantid.properties
file with logging properties whenmvesuvio config
is runTableWorkspaces
in a pretty formatIMPORTANT NOTE: Because Windows has a file lock that prevents a file from being written to when its open by another process, it's not as simple as doing
open()
in line https://github.com/mantidproject/vesuvio/pull/153/files#diff-04c5f47f75adcdbf7d20a33e3fc04fe2a2d231e11f9a843feb830375222c19c4R89, so I talked we Anna and we decided to go ahead with this PR and fix the functunality on Windows later.To test:
Code review
+
Due to the differences in behaviour depending on the OS, manual testing should be done on IDAaaS and on Windows:
IDAaaS:
Create and sctivate a test environment with mantidworkbench installed:
pip install -e .
mvesuvio config
~/.mvesuvio/analysis_inputs.py
BackwardAnalysisInputs
change the fieldrun_this_scattering_type = True
FowardAnalysisInputs
change the fieldnumber_of_iterations_for_corrections = 1
mantid.log
file was created in the working directory.~/.mvesuvio/analysis_inputs/bckwd_fwd_gauss.log
and check that this is the same log but only containing the relevant information (fit of each spectra and tables)~/.mvesuvio/analysis_inputs/bckwd_fwd_gauss.log
contains only the last routine that was run (look at the times of the output lines to check this)Windows:
Create and activate a test environment with mantidworkbench installed:
~/.mvesuvio/analysis_inputs/bckwd_fwd_gauss.log
contains the logs of both routines, and the file is not reset to include only the very last routine that was run.Fixes #145 .