Develop #5
Open
Develop #5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…command line for input file, defaults to initial_atm_vMCMC_2024.txt. Began to work on a grid script.
…dified model to accept command line input, but will still run on default atmosphere input file if no arguments are given.
…all output files into relevant subdirectories under results/run1. Changed line 2482 in all model versions to new expression
…nting GRT computations in GRTs.py, 17 out of 19 reactions implemented.
…plemented new module, computing moles of each species and implementation of boundary conditions.
…y (currently still fails on 4 / 29 equations)
… model.py now computes initial values for f1 - f29.
… file and model.py.
…gmoidal_penalty function to allow array input
… Runs, but a test run gives a small deviation from the result in Young's model.
…lemented small code in testscript.py for sampling the objective function to estimate cost using smoothTriangle function.
…naries as input for cases where both global parameters (ie. params not varied in a model run) are needed alongside variable input. Made function arguments more descriptive.
…function initial. Worked on troubleshooting mean cost estimation against Young model; ongoing.
…ing issues between codes.
…ons. Implemented unit tests for activities and mean cost estimation.
…odel function. Removed check for metal stoichiometry in utilities/moles_in_system as they are definitionally mono-atomic
robsidian
approved these changes
Feb 24, 2025
There was a problem hiding this comment.
Well done, I don't really have any points of improvement. Keep up the good work!
Re: objective function - The cost function stuff is just used to get a general standard deviation estimator that the dual annealing function uses for its statistics. You should try testing how much the results of dual annealing depend on the value of T_estimate (within the range you get from samples of the cost function). If it's not significant, ignore it. If it is, test increasing n_iters until you get a large enough sample size to converge to the actual distribution.
In the objective function etc, why is the pressure variable called P_penalty? This is the value before the penalty is applied, right?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's important to note that while the two codes now agree on the mean / standard deviation of the objective function, this only applies when the models are given the same random seed and number of iterations. The two models behave similarly, but even 10,000 iterations is not sufficient for the mean estimate of the objective function to converge to a solution that both models agree on when given random input. The estimation of the objective function's mean and standard deviation seems sketchy to me, so while the models do now share the same behaviour, I would want to revisit this part of the code later and see if what is happening actually makes sense.