-
Notifications
You must be signed in to change notification settings - Fork 11
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
Refactor music box configuration file options #220
Comments
Actually, we might consider storing all of the mechanism information in this file as well. That would remove the need to write any files out. We would then have to update the musica API to take json objects, but that is already a goal we have anyways so that we push all of the parsing down to musica. @mattldawson would this be a good time to do that or do you think it's too soon? |
I like that idea. Do we need to make any changes to the |
@mattldawson I don't belive so. |
Doing just CSV vs. JSON in this ticket; leave choice of solver (Rosenbrock and others) to another issue. |
This is also related to #260, but only indirectly |
The code base has changed substantially since October. I will re-synch my obsolete dev branch with the latest main. |
Here is the in-progress branch for this issue: |
My initial conditions for dev/test look like this in the JSON:
|
Since "Species configurations are only read from the initial conditions section", then it follows that the chemical species section of my_config.json will be removed and no longer supported. Delete:
|
@mattldawson and @K20shores :
Those are reaction rates, not initial species concentrations. That CSV files looks like this:
Do you intend for us to read the same file twice, one time looking for prefixes like ENV and CONC, and then read it a second time for prefixes like EMIS and PHOTO? We could do that, but I'm thinking it would be easier to support two JSON keywords instead, like: |
The flow tube example also uses "initial conditions" for the reaction rates: The reaction rates look like this: |
I would lean toward allowing any initial conditions (rates, concentrations, temperature, pressure, etc.) to be in one file, or multiple files if a user prefers. Something like:
or
What do you think? |
Thanks @mattldawson. The python code will not know any significance to the filenames, and we could have filepaths like: I do like the idea of not tossing any and all initial conditions into the same CSV file, and letting the researcher pick her own filenaming conventions to reflect her current topic of research. So:
Multiple passes would make the code more modular but slower. |
Initialization of concentrations involves a large number of floating-point numbers, and I believe that specification would be technically better in tabular form (CSV) instead of JSON. We have a JSON mechanism to include CSV file, rather than inserting many values into the JSON configuration. The CSV file would be better for our users to manage and review.
I know that we are retaining compatibility with the Fortran MusicBox. I recommend that we move forward soon with changes that will increase CSV usage through our product suite.
Originally posted by @carl-drews in #214 (review)
Acceptance criteria
Ideas
or, inplace conditions
--update-config
or--fix
so that when you givemusic_box -c my_config.json --fix
, we overwrite the old one, and maybe copy the origional to my_config.json.oldThe text was updated successfully, but these errors were encountered: