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.

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

Feat/518 calculate eal #531

Merged
merged 100 commits into from
Sep 10, 2024
Merged

Feat/518 calculate eal #531

merged 100 commits into from
Sep 10, 2024

Conversation

sahand-asgarpour
Copy link
Contributor

@sahand-asgarpour sahand-asgarpour commented Jul 17, 2024

Issue addressed

Solves #518

Code of conduct

  • I HAVE NOT added sensitive or compromised (test) data to the repository.
  • I HAVE NOT added vulnerabilities to the repository.
  • I HAVE discussed my solution with (other) members of the RA2CE team.

What has been done?

Explain how you addressed the resolution of the related issue, what choices you made and why.

Checklist

  • Code is formatted using our custom black and isort definitions.
  • Tests are either added or updated.
  • Branch is up to date with master.
  • Updated documentation if needed.

Additional Notes (optional)

Add any additional notes or information that may be helpful.

…ated. risk_calculation_mode/year added to the analysis_config
… that have any EV or RP columns with aggregate_wl_suffix of above aggregate_wl.
…instead of INVALID. The latter used to give error.
@sahand-asgarpour sahand-asgarpour added enhancement New feature or request Launch labels Jul 25, 2024
Copy link
Contributor

@ArdtK ArdtK left a comment

Choose a reason for hiding this comment

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

I did look at the changes since my last review. I couldn't check when Carles did his last review.

tests/network/network_config_data/test_get_wl_prefix.py Outdated Show resolved Hide resolved
ra2ce/analysis/losses/losses_base.py Outdated Show resolved Hide resolved
ra2ce/analysis/losses/losses_base.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@ArdtK ArdtK self-requested a review August 13, 2024 15:29
Copy link
Collaborator

@Carsopre Carsopre left a comment

Choose a reason for hiding this comment

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

That the tests give now a timeout should be an indication that something has not been done correctly (performance sinkhole). But in general the remarks I sent have been addressed.

ra2ce/analysis/losses/losses_base.py Outdated Show resolved Hide resolved
ra2ce/analysis/losses/losses_base.py Show resolved Hide resolved
@sahand-asgarpour
Copy link
Contributor Author

That the tests give now a timeout should be an indication that something has not been done correctly (performance sinkhole). But in general the remarks I sent have been addressed.

Thanks for the update. I do not know what is not implemented correctly. Locally the tests that lead to TC timeout error run successfully.

@sahand-asgarpour sahand-asgarpour self-assigned this Sep 4, 2024
@sahand-asgarpour sahand-asgarpour added this to the Launch 2024 milestone Sep 4, 2024
* fix: lanes added

* feat: raise error if not all attributes_to_include in the dgf

* feat: reverted the raise error if not all attributes_to_include in the dgf

* feat: function created and used in nut to update rfid_c

* feat: function used in osm_network_wrapper in nut to update rfid_c

* Update ra2ce/network/networks_utils.py

chore: inline comments changed

Co-authored-by: Carles S. Soriano Pérez <carles.sorianoperez@deltares.nl>

* chore: apply the comments on a previously made function + using deepcopy()

* chore: use of deepcopy is corrected

---------

Co-authored-by: Carles S. Soriano Pérez <carles.sorianoperez@deltares.nl>
@sahand-asgarpour sahand-asgarpour merged commit 2882876 into master Sep 10, 2024
3 checks passed
@sahand-asgarpour sahand-asgarpour deleted the feat/518-calculate-eal branch September 10, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calculate EAL
4 participants