Tickets/sitcom 1131 #2
Conversation
I corrected a few typos in the texts
…tebook while building the Tech Note
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
index.rst
Outdated
|
|
||
| .. Metadata such as the title, authors, and description are set in metadata.yaml | ||
| This technote uses data from the VMS, dc accelerometers and IMS to estimate the effect of the Fan Coil Units (FCUs) are likely to have on image quality |
There was a problem hiding this comment.
Could you replace VMS with Vibration Monitoring System (VMS)?
There was a problem hiding this comment.
Same with IMS and Independent Measurement System (IMS)?
index.rst
Outdated
|
|
||
| This technote uses data from the VMS, dc accelerometers and IMS to estimate the effect of the Fan Coil Units (FCUs) are likely to have on image quality | ||
| Primarily we use the Vibration Monitoring System (VMS) of the M1M3 for more information see the VMS confluence page (link here). | ||
| The VMS data are stored in a single hdf5 data file (/sdf/data/rubin/shared/mtm1m3_test_files/vms_data/2023/12/M1M3-2023-12-07T00:00.hdf) acquired on 2023/12/07. A description of the |
There was a problem hiding this comment.
Could you put single-back-quotes before and after the path to convert it into code-style? This formatting can improve readability. Something like this:
/sdf/data/rubin/shared/mtm1m3_test_files/vms_data/2023/12/M1M3-2023-12-07T00:00.hdf
There was a problem hiding this comment.
single back quotes gave a ugly result so we put the text in a block of code
index.rst
Outdated
|
|
||
| **This technote is a work-in-progress.** | ||
| This technote uses data from the VMS, dc accelerometers and IMS to estimate the effect of the Fan Coil Units (FCUs) are likely to have on image quality. |
There was a problem hiding this comment.
Small suggestion:
This technical note analyzes data from the VMS, dc accelerometers, and the IMS to assess how the operation of Fan Coil Units (FCUs) might impact image quality.
index.rst
Outdated
|
|
||
| Data quality | ||
| ============ | ||
| We have checked that the TMA was not moving during this test (Azimuth: -25.95 deg - Elevation: 89.95 deg) and that the mirror was raised (MTM1M3.logevent_detailedState == ACTIVEENGINEERING) |
There was a problem hiding this comment.
I recommend adding single-back-quotes here too:
MTM1M3.logevent_detailedState == ACTIVEENGINEERING
There was a problem hiding this comment.
Same as above --> we made a block of code
index.rst
Outdated
| We have checked that the TMA was not moving during this test (Azimuth: -25.95 deg - Elevation: 89.95 deg) and that the mirror was raised (MTM1M3.logevent_detailedState == ACTIVEENGINEERING) | ||
| with Hard Point in standby mode. | ||
|
|
||
| We also checked that during the whole duration of the test, the Force Balance System was turned off. |
There was a problem hiding this comment.
I would simplify this sentence:
We also checked that the Force Balance System was turned off during the whole duration of the test.
There was a problem hiding this comment.
I added a few comments but, in overall, this is very good. As I was reading, I was wondering if the ~ 40Hz could be associated with the fans themselves. It seems that 2550 RPM is equivalent to 42,5 Hz. On top of that, I wonder if we do not see the RPM reflected in the PSD. Wouldn't that be expected?
|
Thank you very much for the review. We added a comment about the peak at 42 Hz appearing in some of the plots when fans run at 100% speed. It is difficult to be sure of what we are seeing here but as you say it looks pretty natural to see this peak. |
|
My final recommendation after running the notebook locally would be to try to have the x-labels with the time stamps with a consistent label formant. The plot produced right before the section "PSD Analysis" section has a good format. Another option, maybe for the future, would be to define a reference time and convert the x-axis into seconds. This is easier to work with and, sometimes, easier to read. Of course, this means that we need to print this reference time somewhere. But you are already doing that in the titles. |
|
Thanks for the review. |
…-101 into tickets/SITCOM-1131
This is a Pull Request to review both the Tech. Note and the associated notebook.
Tech Note has been read by Doug Neil.