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

Merge the Merge #1086

Merged
merged 1,667 commits into from
Sep 19, 2024
Merged

Merge the Merge #1086

merged 1,667 commits into from
Sep 19, 2024

Conversation

FabianHofmann
Copy link
Collaborator

@FabianHofmann FabianHofmann commented Aug 16, 2024

This PR merges the pypsa-eur-sec workflow with (mostly all) its functionality. As discussed internally, here is the first draft which you should not expect to run immediately, but this should give a solid basis to resolve remaining discrepancies.

The merge does not include sec functionalities from the make_summary and plot_summary scripts. To more things more efficient, I would suggest we keep the pypsa-earth standards in these scripts for now and rewrite/extend them later with the pypsa.statistics module which automatically processes sector-coupled networks (as well as power networks only of course).

I had to add licenses of incoming scripts to AGPLv3, hope that is okay.

Checklist

  • I consent to the release of this PR's code under the AGPLv3 license and non-code contributions under CC0-1.0 and CC-BY-4.0.
  • I tested my contribution locally and it seems to work fine.
  • Code and workflow changes are sufficiently documented.
  • Newly introduced dependencies are added to envs/environment.yaml and doc/requirements.txt.
  • Changes in configuration options are added in all of config.default.yaml and config.tutorial.yaml.
  • Add a test config or line additions to test/ (note tests are changing the config.tutorial.yaml)
  • Changes in configuration options are also documented in doc/configtables/*.csv and line references are adjusted in doc/configuration.rst and doc/tutorial.rst.
  • A note for the release notes doc/release_notes.rst is amended in the format of previous release notes, including reference to the requested PR.

pre-commit-ci bot and others added 30 commits May 29, 2024 17:04
* fix TypeError

* Update scripts/prepare_gas_network.py

Co-authored-by: Davide Fioriti <[email protected]>

---------

Co-authored-by: Davide Fioriti <[email protected]>
@FabianHofmann
Copy link
Collaborator Author

@ekatef great review, thanks. The ppm version will be updated today to fix all the URLs, this is the last Todo on the list. After that is fixed, we can merge.

@ekatef
Copy link
Member

ekatef commented Sep 16, 2024

@FabianHofmann perfect, thanks a lot!

As a final check, we have to make sure that PyPSA-Earth is working in its state before the merge and cross-check that PyPSA-Earth-Sec is ready for merge.

@davide-f @hazemakhalek can you please review the current status from the bigger-picture perspective 🙏🏽

To my understanding, the following parts should be addressed in PyPSA-Earth:

  • Bugfix for csp buses: add buses one for generator #1076 should be merged (apologies for creating a merge conflict but this part is crucial for the pre-merge functionality);
  • a short note added to the documentation and README on how to use the model in the state before the merge;
  • ideally a minor release to issue before the merge.

Any opinions on that? 🙂

@FabianHofmann
Copy link
Collaborator Author

@ekatef @davide-f everything is ready here :) feel free to push the button

@pz-max
Copy link
Member

pz-max commented Sep 17, 2024

@ekatef @davide-f everything is ready here :) feel free to push the button

Sounds great! It would be great to get this in before Fabian is out of office. So we can quickly fix any potential issues that might arise. Can we prio this merge @ekatef @davide-f @hazemakhalek ?

@ekatef
Copy link
Member

ekatef commented Sep 17, 2024

@ekatef @davide-f everything is ready here :) feel free to push the button

Sounds great! It would be great to get this in before Fabian is out of office. So we can quickly fix any potential issues that might arise. Can we prio this merge @ekatef @davide-f @hazemakhalek ?

Thanks @FabianHofmann for introducing the final changes 😄 Looking forward to see your contribution merged. Thanks @pz-max for support 🙂

As discussed before, the points left are adding the notes to the README and prepare a minor release. I'll try to tackle that during this week but there is clearly some shortage in the resources... Sorry for that, and any support here would be very much appreciated.

As a comment, the merge is a result of join efforts. Both Hazem and Davide are aware of the importance this work and are doing their best to assist it. So, I'd recommend to avoid increasing the pressure which still is high enough.

In any case, the Merge of Merge is a really great work, and we'll for sure finalise in in some days. Really looking forward to it 🚀

@FabianHofmann
Copy link
Collaborator Author

@ekatef I have added a note in the readme on how to run a previous model. Also, happy to push a tag on the main branch. Probably v0.4.1 would be a good choice as not too many things have changed since 0.4.0.

@davide-f
Copy link
Member

davide-f commented Sep 17, 2024

Great!
I don't expect code changes here and on my side we can call it done :D 🦸

In agreement to past discussions, the next things to do are:

  • quickly merge minor open prs
  • release a version v0.4.1 of existing main to establish a latest power model , before officially moving towards a sectoral model
  • merge this PR
  • create a new release v0.5.0

I'll have some time tomorrow, but can't do anything today.

Fabian you have done great and functionality wise I believe this is finalized.
Tomorrow I can take care of the first two points.
If relevant questions appear regarding the existing prs I'll notice but I don't expect that.

Probably we can setup the goal to finalize no later than the next monthly meeting (next week), aiming to do v0.5.0 end of this week, while keeping few days for possible buffer.

What do you think? @ekatef @FabianHofmann @hazemakhalek

@ekatef
Copy link
Member

ekatef commented Sep 17, 2024

Thank you so much @FabianHofmann @davide-f!

@davide-f it sounds like a plan 🙂 Happy to support with those two points tomorrow.

README.md Outdated
**PyPSA-Earth is the first open-source global energy system model with data in high spatial and temporal resolution.** It enables large-scale collaboration by providing a tool that can model the world energy system or any subset of it. This work is derived from the European [PyPSA-Eur](https://pypsa-eur.readthedocs.io/en/latest/) model using new data and functions. It is suitable for operational as well as combined generation, storage and transmission expansion studies. The model provides two main features: (1) customizable data extraction and preparation scripts with global coverage and (2) a [PyPSA](https://pypsa.readthedocs.io/en/latest/) energy modelling framework integration. The data includes electricity demand, generation and medium to high-voltage networks from open sources, yet additional data can be further integrated. A broad range of clustering and grid meshing strategies help adapt the model to computational and practical needs.
**PyPSA-Earth: A Global Sector-Coupled Open-Source Multi-Energy System Model**

PyPSA-Earth is the first open-source global energy system model with high spatial and temporal resolution. Originally it was derived from the European [PyPSA-Eur](https://pypsa-eur.readthedocs.io/en/latest/) model using new data and functions which provide capabilities for modelling the world energy system or any subset of it, enabling large-scale collaboration and transparent analysis for a sustainable energy future. It is suitable for operational studies, as well as expansion studies on combined generation, storage and transmission accounting for cross-sectoral interactions. The model provides two main features: (1) customizable data extraction and preparation scripts with global coverage and (2) a [PyPSA](https://pypsa.readthedocs.io/en/latest/) energy modelling framework integration. In particular, the data includes energy demand, generation and medium to high-voltage networks from open sources, yet additional data can be further integrated. A broad range of clustering and grid meshing strategies help adapt the model to computational and practical needs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would the "sector-coupled" wording in the description on line 32 as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

The description of the data of functionality (1) should be expanded to include the data from the sector-coupled model

Copy link
Member

Choose a reason for hiding this comment

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

Have added a small fix but agree that the description can be definitely improved further to present cross-sectoral capabilities in a more detailed way

@hazemakhalek
Copy link
Collaborator

Great! I don't expect code changes here and on my side we can call it done :D 🦸

In agreement to past discussions, the next things to do are:

* quickly merge minor open prs

* release a version v0.4.1 of existing main to establish a latest power model , before officially moving towards a sectoral model

* merge this PR

* create a new release v0.5.0

I'll have some time tomorrow, but can't do anything today.

Fabian you have done great and functionality wise I believe this is finalized. Tomorrow I can take care of the first two points. If relevant questions appear regarding the existing prs I'll notice but I don't expect that.

Probably we can setup the goal to finalize no later than the next monthly meeting (next week), aiming to do v0.5.0 end of this week, while keeping few days for possible buffer.

What do you think? @ekatef @FabianHofmann @hazemakhalek

Okay, the next things to do sound good to me, I can also do the same. I implemented a small fix in the sector-coupled version, it's issued now as a PR. We can also have a minor release to include that before the merge.

I added some minor comments to the readme, but otherwise everything looks good!

README.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -128,6 +140,17 @@ There are multiple ways to get involved and learn more about our work. That's ho
Java HotSpot(TM) 64-Bit Server VM (build 25.341-b10, mixed mode)
```

## Running the model in previous versions

The model can be run in previous versions by checking out the respective tag. For instance, to run the model in version 0.4.0, which is the last version before the repo `pypsa-earth-sec` was merged, the following command can be used:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for completeness, we can also refer the last version of the sec version in the other repo. We will keep it for some time.

@ekatef
Copy link
Member

ekatef commented Sep 19, 2024

Many thanks @hazemakhalek for finding time to do a review. That helps a lot.

A re-cap of the todays meetings:

  1. There are still some fixes which may be wanted in the power-only model. However, they are not urgent, and we can find an approach to apply them even after the merge.
  2. This PR is ready to go, and there seems to be no obstacle to merge it now.
  3. After merging this PR, there will be need to take some time to introduce any additional fixes which may be needed before releasing v0.5.0. In particular, we have to remember to add a reference to a fresh release of the cross-sectoral model.

Merging in a couple of minutes 🙂

@ekatef ekatef merged commit a898746 into main Sep 19, 2024
4 checks passed
@ekatef
Copy link
Member

ekatef commented Sep 19, 2024

Merged. Thanks for the amazing contribution @FabianHofmann!
Great work @hazemakhalek @davide-f @pz-max 😄

As a reminder, please feel free to track status and add fixes before the merge-release.

@hazemakhalek my feeling is that it would be great to revise further presentation of the model to better reflect cross-sectoral capabilities. Absolutely agree with your comments to README, and we added some fixes to address them. But we definitely don't have your expertise, and the changes are quite shallow. Please feel free to deepen the description as you feel appropriate.

finozzifa added a commit to open-energy-transition/pypsa-earth that referenced this pull request Sep 24, 2024
* Update release_notes.rst (pypsa-meets-earth#1104)

* Merge pull request pypsa-meets-earth#1086 from merge-pyspa-earth-sec

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Comment add_brownfield to bypass linter

* Add myopic test

* Revise irena and minor fixes

* Revise test name

* Add existing_heating

* implement review suggesstions

* bug fix in solve_network with reference case

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update submodule

* Add zeros for missing entries - aluminium

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* adding missing templates

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix MissingOutputException

* fix TypeError (pypsa-meets-earth#321)

* fix TypeError

* Update scripts/prepare_gas_network.py

Co-authored-by: Davide Fioriti <[email protected]>

---------

Co-authored-by: Davide Fioriti <[email protected]>

* solve pandas deprecations

* Add Params for Rule add_export.py

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to Rule build_base_energy_totals

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to Rule build_base_industry_totals

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to Rule build_cop_profiles

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to Rule build_heat_demand

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to build_industrial_distribution_key

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to build_industry_demand

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params and urban_percentage effect all Rules

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to build_ship_profile

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params build_solar_thermal_profiles

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to build_temperature_profiles

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to copy_config

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params make_summary

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add to override_respot + panning_horizons wildcard

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update Snakefile

* Update Snakefile

* Add Params to prepare_airports

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to prepare_gas_network

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Add Params to prepare_db and energy_totals

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update Snakefile

* Update add_export.py

* Add Params to build_population_layout

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* enhance the industry scripts and adapt the fuel aggregation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove legacy EUR code

* remove legacy params form config

* remove legacy params form config

* Update build_base_industry_totals.py

* Update build_base_energy_totals.py

* omit double transpose

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove plotting from workflow

* remove plotting from prepare_gas_network

* Test routine: add Makefile
CI: consolidate ci.yaml files

* CI: bump cache number, remove os prefixes and labels
env: remove ipopt restriction

* Makefile: add tutorial yaml as additional config

* snakefile: use config.default as basis
ci: use tutorial config as secondary config

* snakefile: fix databundle config path

* debug: print out downloaded files

* snakefile: use renewables as bases for used cutouts

* Include scenario management

* Add scenario in tutorial

* Update submodule

* Add shared_cutouts

* Fix missing RDIR_PE

* fix IndexError (pypsa-meets-earth#326)

* fix IndexError

* '.' gets only added when len(id) > 3

* fix with layer_id

* Update scripts/prepare_gas_network.py

Co-authored-by: Davide Fioriti <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix index error when layer_id == 0

---------

Co-authored-by: Davide Fioriti <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* solve_network: modularize solver_options to allow for incremental config changes
config: harmonize and apply yaml linting

* refactor configs: boil down to effective diff in tutorial yaml and scenarios yaml

* test: remove config no_progress

* ci: disable for windows

* helpers: use copy_default_files

* config: make config.default + config.tutorial match old config.tutorial

* solve_network: fix options assignment in prepare

* add build_heating_distribution

* Update licensing

* Set location to Earth

* Update cluster pop

* Fix typos

* Bugfix to skip h2 pipelines with missing buses

* Bugfix h2_network loc

* reintroduce config.tutorial.yaml as basis (possibly revert this commit later to reenable config.default)

* doc: update testing documentation [skip ci]

* add comments to makefile

* Update myopic test

* Bugfix none location in build_industrial_database

* Revise run_test myopic name

* revert transpose

* delete plot_network_eur.py

* minor bug in build_base_energy_totals

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update build_base_industry_totals.py

* adapt build_base_energy and build_base_industry to params

* add missing param in snakefile

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Revise myopic test file

* fix param in snakefile

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* reset config.pypsa-earth.yaml

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update Snakefile

* handle exception to avoid reference before assignment error

* Add different scenario name for subworkflow

* Fix myopic test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update config.test_myopic.yaml to include new parameter

* replace snakemake subworkflow by submodule; fix path relations

* fill NaN without international bunkers

* remove factor for gas emissions from residential and services sector

* account for multi-country-cases

* snakefile: replace rdir by resdir for compatibility with pypsa-earth

* Update build_base_industry_totals.py

* Update Snakefile

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update build_base_industry_totals.py

* modified config to fix nan objective value

* print objective value to terminal

* fix for urls not working

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* added license for datasets

* Update README.md

* Update README.md

* Final README adaptations before v0.2 release (pypsa-meets-earth#364)

* Adding the updated network representation figure

* Delete docs/SCPE_v0.2.pdf

* Include new network representaion figure as png

* Update README.md

* Embed the new network figure in the readme

* Delete outdated incomplete network configurations

* updated pypsa-earth submodule to v0.4.0

* adjust modular solver options

* resolve global paths

* workflow: use global root directory to avoid recursive upwards chdir

* update pypsa-earth commit

* revert changes to config.default and config.tutorial priority

* udpate pypsa-earth

* consolidate CI yaml

* yamllint: align formatting and config

* Snakefile: fix SDIR and RESDIR path ending

* ci.yaml include git submodule

* fix duplicated "/"

* remove config.pypsa-earth.yaml in favour of actual pypsa-earth default config

* remove pypsa-earth.config from copy_config

* ci: use one core to better track log

* test: use more core

* make yamllint compatible

* build_renewable_profiles: use local client in order to suppress verbose dask output

* update submodule

* config: consolidate clustering key
snakefile: fix cost retrieval

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* solve_networks: fix network reference for sector coupled case

* address review comments

* config.myopic.yaml: revert country order again

* Update README.md

Co-authored-by: Davide Fioriti <[email protected]>

* address davides comments (first round)

* consolidate current working directory; considate helpers scripts

* fix reference to helpers script; fix fiona version

* env: update ppm version

* build_industrial_database: make retrieval robust against restrictive permissions

* follow up: consolidate restrictive url retrievals

* helpers: reinsert gadm functions from pypsa-earth-sec due to discrepancies (solve those later)

* env: temporarily install new earth osm version from pypi

* ci: restrict ipopt for windows

* env: follow up

* ci: roll back and disable windows

* _helpers: try make content_retrieval more robust

* Update README.md

Co-authored-by: Ekaterina <[email protected]>

* config: bump version; add `allow_scenario_failure` flag; remove `base` cutout comment

* Snakefile: make cutout path consistent for sector-coupled version
config.default: make hydro extedable again

* helpers: fix numpy random usage

* properly remove submodule

* add version tag to all configs

* env: update powerplantmatching

* add missing config version tag

* update powerplantmatching

* follow up: move ppm installation to pip while conda is not out

* readme: add description on running previous models

* helpers: add HTTPError as allowed exception

* config: remove lifetime from config.default

* harmonize cost calculation in sector coupled model

* bump version tag in readme [skip ci]

* update README to account for Hazem's comments

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: davide-f <[email protected]>
Co-authored-by: Hazem-IEG <[email protected]>
Co-authored-by: Davide Fioriti <[email protected]>
Co-authored-by: Hazem <[email protected]>
Co-authored-by: Eddy Jalbout <[email protected]>
Co-authored-by: finozzifa <[email protected]>
Co-authored-by: Anton Achhammer <[email protected]>
Co-authored-by: Anton Achhammer <[email protected]>
Co-authored-by: Fabrizio Finozzi <[email protected]>
Co-authored-by: Eddy-JV <[email protected]>
Co-authored-by: energyLS <[email protected]>
Co-authored-by: cpschau <[email protected]>
Co-authored-by: Emmanuel Bolarinwa <[email protected]>
Co-authored-by: Ekaterina <[email protected]>

* code: re-add copy_default_files

* code:fix imports

* code:fix arguments in build_osm_network

* code:geo_crs value as explicit argument

* code:remove unnecessary arguments

* code: modify the demand in scripts/build_base_energy_totals.py

* code: modify the demand in scripts/build_base_energy_totals.py - 2

* code: modify the demand in scripts/build_base_energy_totals.py - 3

* code: modify the demand in scripts/build_base_industry_totals.py

* code: modify the demand in scripts/build_base_industry_totals.py and scripts/build_base_energy_totals.py

* code:provide missing arguments in locate_bus

* code: add missing arguments in prepare_sector_network

* code: remove tuple

* code: remove tuple

* code:remove environment.mac and update test_build_powerplants

---------

Co-authored-by: Davide Fioriti <[email protected]>
Co-authored-by: Fabian Hofmann <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: davide-f <[email protected]>
Co-authored-by: Hazem-IEG <[email protected]>
Co-authored-by: Hazem <[email protected]>
Co-authored-by: Eddy Jalbout <[email protected]>
Co-authored-by: Anton Achhammer <[email protected]>
Co-authored-by: Anton Achhammer <[email protected]>
Co-authored-by: Eddy-JV <[email protected]>
Co-authored-by: energyLS <[email protected]>
Co-authored-by: cpschau <[email protected]>
Co-authored-by: Emmanuel Bolarinwa <[email protected]>
Co-authored-by: Ekaterina <[email protected]>
@hazemakhalek
Copy link
Collaborator

hazemakhalek commented Oct 8, 2024

Hey @ekatef,

Just back in office after some time out. I see you no history of pypsa-earth-sec commits is available. Did we do a squash and merge for the whole repo? We agreed to keep the history.

@ekatef
Copy link
Member

ekatef commented Oct 8, 2024

Hey @ekatef,

Just back in office after some time out. I see you no history of pypsa-earth-sec commits is available. Did we do a squash and merge for the whole repo? We agreed to keep the history.

Hey @hazemakhalek, you are right... It seems I didn't have a point in a check list for a type of the merge and got distracted with the other point which needed alignment. Apologies for that and thanks a lot for catching. As agreed, the solution is to revert the merge commit and re-merge.

Also, it appears that it could be a very good idea to have a check-list for maintenance and contributions.

nilgpe pushed a commit to nilgpe/pypsa-earth that referenced this pull request Oct 11, 2024
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.