-
Notifications
You must be signed in to change notification settings - Fork 12
added _irradschedule reading from input file #60
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
added _irradschedule reading from input file #60
Conversation
|
Just wondering there are any changes needed on this PR |
Sorry Jon, I'll try and take a look this weekend. I think it's fine though |
pypact/input/inputdata.py
Outdated
| next_line = next(lines, None) | ||
| if next_line and next_line.strip().startswith("TIME"): | ||
| time_parts = next_line.strip().split() | ||
| if len(time_parts) != 3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if it's as simple as that.
I need to re-read the documentation but I think you can have situations like:
TIME 0.1 YEARS ATOMS
and
TIME 60
and even when the ATOMS is on the next line, such as
TIME 0.1
ATOMS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confirming this is a situation I've just seen in the new test file I've added
FLUX 1.116E+10
ATOMS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these cases are now covered and represented in the test suite input files
| fluxAmp = float(parts[1]) | ||
|
|
||
| # Ensure FLUX is set to a non-zero value before the first irradiation step | ||
| if not flux_set and fluxAmp <= 0.0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can do just decay in F-II without any ZERO keyword, but this needs to be confirmed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking into this and I think yes FISPACT does decay without the ZERO keyword. Just setting the FLUX to 0.0 and the material decays. What ZERO does in addition to setting FLUX to 0.0 is to switch from irradiation to cooling schedule. So I've added a build up of cooling schedule to this PR as well. Sorry for the scope creep.
https://fispact.ukaea.uk/wiki/Keyword:ZERO
https://fispact.ukaea.uk/wiki/Keyword:FLUX
| FLUX 1100000000000000.0 | ||
| TIME 300.0 SECS | ||
| FLUX 42.0 | ||
| TIME 200.0 SECS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're going to need a lot more test cases for this.
Unfortunately, the system tests are not available, but perhaps the workshops can provide some valuable test cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 5 test input files from the workshop and added them to the test for irradiation and cooling schedule checking ✔️
|
Thanks for the review and the pointers |
|
I think this is ready for another review |
|
🎉 CI passed |
|
Alright I guess this works. LGTM. |
|
Thanks for the merge, I'm already making use of this feature over here jbae11/openmc_activator#11 |
|
one thing I noticed when using the feature is that we get a (0,0) value which remove with this command cleaned_irradschedule = [item for item in ff._irradschedule if item != (0.0, 0.0)]\n", let me know if you want a PR to remove the (0,0) value at this end? |
Since you're both the author and user of this part of the code, I'll let you decide how you want this to behave. Feel free to open a PR if you want to fix it. |
@thomasms any chance you have time to review this PR that solves the above issue #63 I promise it is a small PR |
The irradiation schedule made from FLUX and TIME keywords can also be parsed from the inputfile
For this I've added special handling to look for the ZERO keyword as I believe that ends the irradiation schedule
I have also special handling for the FLUX 0.0 input as I believe this does not need following with a TIME keyword
I have added some extra FLUX and TIME keywords to the test input file to check multiple steps are read correctly
I think if this is successful then I only need to add cooling steps and I have all the input file reading functionalities needed for the foreseeable needs