Closed stefsmeets closed 1 month ago
Got the loop working
Code works, but I get hit with ERROR tests/integration/test_dummy_integration.py::test_dummy_run - Exception: Could not find solution for volatile abundances (max attempts reached)
Any ideas where to look @nichollsh @timlichtenberg?
This usually comes about if the outgassing model (CALLIOPE) is given unreasonable conditions, such as very low temperatures or zero-valued mass inventories. I can see that the T_magma and target masses in the log look reasonable, and this configuration should work fine since we used it in the previous tests.
I would suggest checking that the parameters which are passed to the outgassing code are reasonable. For example, maybe it's being provided with M_mantle=0, or the core fraction is unset, or something along those lines.
Sounds like it could be this bit where I made quite some changes. Do you see anything odd?
EDIT: Looks like there is a typo in the radius EDIT2: That fixed it, thanks for saving me many hours of debugging @nichollsh :1st_place_medal:
No problem! Glad that this works.
Alright, next question for @nichollsh @timlichtenberg, any thoughts about which parameter I should look at? I'm very close to get this to work, but I think I messed up some parameter that affects the surface temp (starts at 3500 instead of 3250ish) which means the transition starts slightly earlier in the current tests.
Actual:
Expected:
Diff:
Could be something to do with the init loops. The way the initial temperature is set differs between the dummy interior and SPIDER (we should think about this more deeply elsewhere - ideally we'd just set T_surf in the config file and that would be accepted by all the interior modules).
The dummy model should enforce an initial temperature by setting T_magma
equal to 3500 during every step of the init phase. This is hardcoded inside the RunDummyInt
function, and applies when IC_INTERIOR==1
, which is true when loop_counter["init"] < loop_counter["init_loops"]
.
If you've changed the looping stuff, maybe that could explain this?
Thanks, the issue was that the surface state was set to 'skin' instead of 'fixed' in the config 👍
Dummy tests are passing now 🚀
@lsoucasse @nichollsh The tests are passing and I think I covered many modules already. Could you help and have a test with some of your favorite modules (spider, zephyrus, janus, etc) to find the last places where the config should be updated?
Sure! I'll run some models by writing some configs "by hand" to see if we get the expected output.
@stefsmeets, when I try to run proteus with the default config it raises an error:
Traceback (most recent call last):
File "/home/n/nichollsh/nobackups/miniforge3/envs/proteus/bin/proteus", line 8, in <module>
sys.exit(cli())
^^^^^
File "/home/n/nichollsh/nobackups/miniforge3/envs/proteus/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
return self.main(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/n/nichollsh/nobackups/miniforge3/envs/proteus/lib/python3.12/site-packages/click/core.py", line 1078, in main
rv = self.invoke(ctx)
^^^^^^^^^^^^^^^^
File "/home/n/nichollsh/nobackups/miniforge3/envs/proteus/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/n/nichollsh/nobackups/miniforge3/envs/proteus/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
return ctx.invoke(self.callback, **ctx.params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/n/nichollsh/nobackups/miniforge3/envs/proteus/lib/python3.12/site-packages/click/core.py", line 783, in invoke
return __callback(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/n/nichollsh/PROTEUS/src/proteus/cli.py", line 75, in start
runner.start(resume=resume)
File "/home/n/nichollsh/PROTEUS/src/proteus/proteus.py", line 92, in start
ValidateInitFile(self.directories, self.config)
File "/home/n/nichollsh/PROTEUS/src/proteus/utils/coupler.py", line 250, in ValidateInitFile
if (config[key_pp] > 0.0) and (config["solvevol_use_params"] > 0):
~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
File "/home/n/nichollsh/PROTEUS/src/proteus/config/_config.py", line 40, in __getitem__
val = conv(self)
^^^^^^^^^^
File "/home/n/nichollsh/PROTEUS/src/proteus/config/_compatibility.py", line 41, in _solvevol_use_params
raise ValueError("Equivalent to `delivery.initial = 'elements'`")
ValueError: Equivalent to `delivery.initial = 'elements'`
Do we still need this function ValidateInitFile()
? Presumably these checks should be made inside the _config/
Python files.
I will pick up validation after, please add any validation related tasks to #200
solvevol_use_params
should be factored out, since it is no longer needed based on your comment in https://github.com/FormingWorlds/PROTEUS/issues/74#issuecomment-2385375968 , can this bit be deleted?
I would say we can delete it, yeah. If you push to this branch I will pull it again and try to run some more tests.
Hi @stefsmeets, I don't have the log print on the terminal anymore with this branch. How can I retrieve it? The log file in the output folder is also empty.
@lsoucasse Which workflow are you running? I turned it off for the dummy toml, because it gives too much spam with pytest.
@stefsmeets another minor issue, now that the solvevol one is fixed:
File "/home/n/nichollsh/PROTEUS/src/proteus/atmos_clim/janus.py", line 54, in InitAtm
raise Exception("Invalid tropopause option '%d'" % config["tropopause"])
The match
statement in this function needs to be changed to match strings, rather than integers for the tropopause type.
For the escape/zephyrus we miss two keys:
I guess the idea is to add them to the class and the compatibility map? Is that all?
The default case seems to run fine now for me. However, the HD63433d case with AGNI crashes.
File "/home/n/nichollsh/PROTEUS/src/proteus/atmos_clim/agni.py", line 160, in init_agni_atmos
elif chem_type >= 2:
^^^^^^^^^^^^^^
TypeError: '>=' not supported between instances of 'NoneType' and 'int'
@lsoucasse It's not immediately clear to me where these come from, because these are not in #74 . Can we add these in a follow-up PR? Together with the omega/pctle if needed?
@lsoucasse It's not immediately clear to me where these come from, because these are not in #74 . Can we add these in a follow-up PR? Together with the omega/pctle if needed?
@stefsmeets , these parameters are specific to escape.toml. But I agree they can be addressed is a separate PR. We'll need to go through all option key anyway to resolve the warnings.
I got default.toml
running without any errors, and tests are also passing. I didn't have a chance to test agni yet.
I sorted out many (all?) of the int -> str conversions.
Follow-up:
Question: are we planning on eventually phasing out the need for this conversion layer? Are we planning to access the variables like config.atmos_clim.tropopause
rather than as config["tropopause"]
? Obviously it's useful in the meantime while we adapt the code to the new config format.
Edit: Ignore this! I can see now the very annoying warning that you describe, which answers this question.
~Question: are we planning on eventually phasing out the need for this conversion layer? Are we planning to access the variables like
config.atmos_clim.tropopause
rather than asconfig["tropopause"]
? Obviously it's useful in the meantime while we adapt the code to the new config format.~Edit: Ignore this! I can see now the very annoying warning that you describe, which answers this question.
I think it is still worth coordinating to address these warnings, maybe module by module.
Thanks for the feedback!
@nichollsh I'm having some issues getting agni to work (need to reinstall agni), but if it works for you and you are happy with the changes, then this PR can be merged.
having some issues getting agni to work
Maybe we can discuss this at the next proteus meeting.
This PR updates the code to use the new style config. To help with the transition, I added a compatibility layer with the old style config, so that a dict lookup (like
OPTIONS['...']
) will still work for the most part, but the idea is to completely move over to the new format.I added a superannoying warning to help with motivation :smiling_imp:
I also updated all the old input tomls to the new format.
The most tricky bits are around the years, which changed from yr -> Gyr, and modules, which changed from integers to enums. Please pay extra attention to those.
Closes #74
TODO