Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Tid #168

Merged
merged 3 commits into from
Jul 7, 2023
Merged

Tid #168

merged 3 commits into from
Jul 7, 2023

Conversation

GabrieleMeoni
Copy link
Collaborator

Description

Summary of changes

  • Added TID modelling by using custom property.

Resolved Issues

Related Pull Requests

@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 Jun 22, 2023 •
edited
Loading

Overall Coverage

Coverage Report
FileStmtsMissCoverMissing
paseos
   __init__.py31294%46, 49
   paseos.py1511491%71–72, 144–145, 198, 213, 231, 240, 258, 289, 297–298, 398–399
paseos/activities
   activity_manager.py43393%46, 76, 159
   activity_processor.py58198%123
   activity_runner.py621084%80–84, 104–113, 122–125
paseos/actors
   actor_builder.py1082181%20–24, 27, 119–124, 248, 296, 318, 328–332, 339–340, 345–346
   base_actor.py1211885%82, 99, 115, 153, 193–195, 224, 233, 251, 268, 272–275, 279, 293, 335
   ground_station_actor.py15380%47–53
   spacecraft_actor.py61198%123
paseos/communication
   get_communication_window.py22291%38, 67
   is_in_line_of_sight.py741580%75, 82, 90, 152–169, 213, 233
paseos/power
   charge_model.py17194%54
   discharge_model.py7271%22, 34
   is_in_eclipse.py39490%28–29, 90–91
paseos/tests
   activity_test.py57395%92, 95–96
   communication_window_test.py58198%198
   eclipse_test.py11191%22
   import_test.py6183%13
   init_test.py8188%16
   line_of_sight_test.py65494%147–150
   thermal_model_test.py30197%63
   visualization_test.py20195%31
paseos/utils
   check_cfg.py611870%31, 40, 46, 62–64, 71, 74–76, 79–81, 84–86, 101, 106
   operations_monitor.py71494%88, 129–132
paseos/visualization
   animation.py18667%27–30, 35, 44
   plot.py9367%28–31
   space_animation.py1952090%135, 159, 162–165, 171, 283, 308–310, 315, 325–326, 330, 365, 371–372, 405, 421–432
TOTAL183116191% 

Tests Skipped Failures Errors Time
30 0 💤 0 ❌ 0 🔥 58.156s ⏱️

@gomezzz gomezzz changed the base branch from main to test-for-notebooks June 22, 2023 13:55
@gomezzz gomezzz changed the base branch from test-for-notebooks to main June 22, 2023 13:56
@gomezzz
Copy link
Collaborator

gomezzz commented Jun 22, 2023

(refreshed diff)

gomezzz
gomezzz reviewed Jun 22, 2023
View reviewed changes
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb Show resolved Hide resolved
gomezzz
gomezzz requested changes Jun 22, 2023
View reviewed changes
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.

see comments ✌️

gomezzz
gomezzz reviewed Jun 26, 2023
View reviewed changes
examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb
@@ -0,0 +1,497 @@
{
Copy link
Collaborator

@gomezzz gomezzz Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Line #11.    from numpy.random import lognormal

unused


Reply via ReviewNB

examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb
@@ -0,0 +1,497 @@
{
Copy link
Collaborator

@gomezzz gomezzz Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Line #13.    from matplotlib import patches

unused


Reply via ReviewNB

examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb
@@ -0,0 +1,497 @@
{
Copy link
Collaborator

@gomezzz gomezzz Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This warns

08:53:51|PASEOS-WARNING| You provided a different starting epoch for PASEOS than the local time of the local actor.starting_epoch will be used.

why?


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not have a clue. I think I use start_date for both sim and scaffolding.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the cell before

Line #5.    sat = ActorBuilder.get_actor_scaffold(name="sat", actor_type=SpacecraftActor, epoch=pk.epoch(0))

Here you define the start time as pk.epoch(0) not start date ;)

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the cell above

Line #5. sat = ActorBuilder.get_actor_scaffold(name="sat", actor_type=SpacecraftActor, epoch=pk.epoch(0))

Here you define the start time as pk.epoch(0) not start date ;)

examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb
@@ -0,0 +1,497 @@
{
Copy link
Collaborator

@gomezzz gomezzz Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can you add x and y labels? :)


Reply via ReviewNB

examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb
@@ -0,0 +1,497 @@
{
Copy link
Collaborator

@gomezzz gomezzz Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

maybe can link to the readme entry on constraint function here in case people don't know what it is?


Reply via ReviewNB

examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb
@@ -0,0 +1,497 @@
{
Copy link
Collaborator

@gomezzz gomezzz Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Line #4.        print("Time:", sat.local_time, "Dose [rad]: ", dose)

Can you change to

 "TID [rad]: ", dose

(it is a bit confusing that you use them interchangeably since dose can also be the dose gotten during one step



Reply via ReviewNB

examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb
@@ -0,0 +1,497 @@
{
Copy link
Collaborator

@gomezzz gomezzz Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Line #18.

there are quite some empty lines, maybe remove some?


Reply via ReviewNB

examples/TID_model_custom_property/Modelling_TID_TNID_example_notebook.ipynb
@@ -0,0 +1,497 @@
{
Copy link
Collaborator

@gomezzz gomezzz Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Line #1.    sim.advance_time(3600 * 24 * 30, 10, constraint_function=constraint_func_TID_failure)

It runs longer now! :) However, differently on my machine. Can you just add a np.random.seed(42) in the top somewhere? :)


Reply via ReviewNB

gomezzz
gomezzz requested changes Jun 26, 2023
View reviewed changes
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.

Just some minor comments, looking good 💪

@gomezzz gomezzz mentioned this pull request Jun 26, 2023
Merged
1 task
gomezzz
gomezzz approved these changes Jun 27, 2023
View reviewed changes
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.

Minor change remaining, feel free to merge after, I think! ✌️ :)

@gomezzz
Copy link
Collaborator

gomezzz commented Jul 3, 2023

@GabrieleMeoni shall we merge?

@gomezzz gomezzz mentioned this pull request Jul 7, 2023
Closed
@GabrieleMeoni GabrieleMeoni merged commit d31d9de into main Jul 7, 2023
2 checks passed
@GabrieleMeoni GabrieleMeoni deleted the tid branch July 7, 2023 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gomezzz gomezzz gomezzz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@GabrieleMeoni @gomezzz