Skip to content

Conversation

@GabrieleMeoni
Copy link
Collaborator

@GabrieleMeoni GabrieleMeoni commented Feb 1, 2024

Description

Summary of changes

  • Merging students' contributions (cleaned by me) to main.

Resolved Issues

How Has This Been Tested?

Refer to the specific tests included in the PR.
Issue #207 shall be taken into account in a separate PR.

…of floats, outputs: numpy arrays (needs to be reviewed in implementation)
…l and magnetic) on actor. started with set_attitude_model and attitude model class as well.
deleted redundant function (which I added myself)
pointing vector function
@GabrieleMeoni GabrieleMeoni requested a review from gomezzz February 1, 2024 15:11
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions
Copy link

github-actions bot commented Feb 1, 2024

Overall Coverage

Coverage Report
FileStmtsMissCoverMissing
paseos
   __init__.py33197%49
   paseos.py1481292%67–68, 145–146, 213, 228, 246, 255, 273, 304, 312–315
paseos/activities
   activity_manager.py43393%46, 74, 157
   activity_processor.py58198%121
   activity_runner.py621084%80–84, 104–113, 122–125
paseos/actors
   actor_builder.py2233385%24–30, 33, 223–225, 252–254, 299–308, 339, 345, 493, 572–573, 588, 624, 671, 702, 714–718, 725–726, 733–734
   base_actor.py1351887%85, 107, 123, 188, 228–230, 251, 260, 277, 298, 304–307, 313, 328, 383
   ground_station_actor.py15380%47–53
   spacecraft_actor.py1081487%65–71, 79–85, 93–99, 198, 253, 262–265
   spacecraft_body_model.py52492%54, 89, 142, 157
paseos/attitude
   attitude_model.py89298%159, 168
   disturbance_torques_utils.py694929%46–222
paseos/central_body
   central_body.py73692%70, 179–180, 188–189, 258
   is_in_line_of_sight.py581771%106–123, 167, 174–187
   mesh_between_points.py35294%71, 79
   sphere_between_points.py26869%35–38, 48–51, 63–64
paseos/communication
   find_next_window.py201525%29–60
   get_communication_window.py231822%35–66
paseos/power
   charge_model.py16194%49
   discharge_model.py7271%22, 34
paseos/tests
   activity_test.py57395%90, 93–94
   attitude_test.py683351%93–129, 137–166
   eclipse_test.py10190%20
   import_test.py6183%13
   init_test.py8188%16
   line_of_sight_test.py62494%143–146
   mesh_test.py119397%118, 139, 147
   thermal_model_test.py30197%61
   visualization_test.py18194%28
paseos/utils
   check_cfg.py611870%31, 40, 46, 60–62, 67, 70–72, 75–77, 80–82, 98, 103
   operations_monitor.py71396%127–130
   reference_frame_transfer.py72889%108, 132, 240–242, 245–247
paseos/visualization
   animation.py18667%27–30, 35, 44
   plot.py9367%29–32
   space_animation.py2152389%108–109, 124, 153–154, 188, 212–219, 225, 335, 358–360, 365, 374–375, 379, 415, 421–422, 453, 467–478
TOTAL265532888% 

Tests Skipped Failures Errors Time
38 0 💤 0 ❌ 0 🔥 1m 6s ⏱️

@GabrieleMeoni
Copy link
Collaborator Author

Issue #207 shall be taken into account in a separate PR.

Copy link
Collaborator

@gomezzz gomezzz left a comment

Choose a reason for hiding this comment

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

Hey! Thanks for the cleaning. A few general comments in addition:

  • Please add short examples to the readme on using these models
  • Please add tests for reference frame conversions

For the rest see individual comments. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we expose these feature to the users?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Logging missing in this file

import numpy as np


def compute_transformation_matrix_eci_rpy(r, v):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this function is not used else, where so probably should be _compute_transformation_matrix_eci_rpy ?

There is no test for this function

assert np.round(sat1.temperature_in_K, 3) == 278.522


def attitude_and_orbit_test():
Copy link
Collaborator

Choose a reason for hiding this comment

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

wrong function name will not be run

Copy link
Collaborator

Choose a reason for hiding this comment

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

wrong filename, has to end with _test or it will not be run

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file was included by mistake. I removed it.

return sentinel2B, maspalomas_groundstation


def main():
Copy link
Collaborator

Choose a reason for hiding this comment

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

will not be run since there is no function test_

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same as above.

Comment on lines +84 to +88
# Pointing vector from sat2 must not be rotated.
assert np.all(sat2.pointing_vector == np.array([-1.0, 0.0, 0.0]))
# Sat2 angular velocity in the body frame must stay zero:
assert np.all(sat2._attitude_model._actor_angular_velocity == np.array([0.0, 0.0, 0.0]))

Copy link
Collaborator

Choose a reason for hiding this comment

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

only checking against trivial values, no actual check if the model produces correct values after e.g. 5 seconds of rotation

@gomezzz
Copy link
Collaborator

gomezzz commented Feb 2, 2024

@rasmusmarak this is the work @GabrieleMeoni was mentioning in our call. Maybe, a good first step for you could be to create a small example notebook branching out of this to try out if you can compute the point on Earth's surface that a satelliteActor is pointing at? Ideally any problems and questions that arise during that can also be used in this PR as feedback. (Maybe @GabrieleMeoni could start by just adding a few sentences to the readme on how to use this to enable this?)

@ghost
Copy link

ghost commented Feb 8, 2024

@rasmusmarak this is the work @GabrieleMeoni was mentioning in our call. Maybe, a good first step for you could be to create a small example notebook branching out of this to try out if you can compute the point on Earth's surface that a satelliteActor is pointing at? Ideally any problems and questions that arise during that can also be used in this PR as feedback. (Maybe @GabrieleMeoni could start by just adding a few sentences to the readme on how to use this to enable this?)

Sounds like a good start, I'll make sure to have a look at some initial notebook example! :-)

@gomezzz gomezzz mentioned this pull request Feb 9, 2024
1 task
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.

6 participants