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

Fix grid_proteus.py #231

Merged
merged 14 commits into from
Oct 25, 2024
Merged

Fix grid_proteus.py #231

merged 14 commits into from
Oct 25, 2024

Conversation

nichollsh
Copy link
Contributor

@nichollsh nichollsh commented Oct 24, 2024

Currently, the grid functionality is broken because it expects the configuration file to be formatted as key=value. This functionality is very useful, so I have made a quick patch to restore it. In the future we should explore #204 and #162 in more detail. I have also included some groundwork for approaching these, such as moving more variables into the Proteus class.

I have written this in such a way that it depends on tomlkit, which requires Python >= 3.11, meaning that PROTEUS now also requires Python>=3.11. Correspondingly, the tests now run for 3.11 and 3.13 rather than 3.10 and 3.12.

Minor things:

  • Added a temperature floor (currently 700 K) for the outgassing calculation, which is set by a new configuration parameter. I found that low temperatures produced by the dummy module can cause problems for CALLIOPE. It's unlikely that this will be needed in a simulation with a 'real' interior model, but it's better than silently crashing.

@nichollsh nichollsh marked this pull request as ready for review October 24, 2024 14:33
Copy link
Member

@lsoucasse lsoucasse left a comment

Choose a reason for hiding this comment

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

Thanks @nichollsh,
It is worth investing efforts in the grid tooling as it will probably be the main way to interact with the code in the future.
I saw you deleted two input config files. It is good to keep only a few of them.

@@ -82,8 +78,13 @@ def __init__(self, *, config_path: Path | str) -> None:
# Loop counters
self.loops = None

# Interior
self.IC_INTERIOR = -1 # initial condition
Copy link
Member

Choose a reason for hiding this comment

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

What is supposed to do this -1 value when passed to the interior models?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a default value that is updated further down in the code. I set it to -1 to indicate that it's a default value.

@nichollsh
Copy link
Contributor Author

Thanks @lsoucasse. I agree we should think carefully about the future work on grid_proteus - maybe this is something we can discuss in a meeting soon.

@nichollsh nichollsh merged commit 78fa766 into main Oct 25, 2024
5 checks passed
@nichollsh nichollsh deleted the grid branch October 25, 2024 08:02
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.

2 participants