Skip to content

Conversation

@shimwell
Copy link
Contributor

@shimwell shimwell commented Apr 22, 2025

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

@shimwell
Copy link
Contributor Author

shimwell commented May 9, 2025

Just wondering there are any changes needed on this PR

@thomasms
Copy link
Member

thomasms commented May 9, 2025

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

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:
Copy link
Member

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

Copy link
Contributor Author

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

Copy link
Contributor Author

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:
Copy link
Member

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.

Copy link
Contributor Author

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
Copy link
Member

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.

Copy link
Contributor Author

@shimwell shimwell May 23, 2025

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 ✔️

@shimwell
Copy link
Contributor Author

Thanks for the review and the pointers
That workshop link is super useful I can see some of the examples are not parsing and failing the tests.
I shall get going on a more compatible parsing

@shimwell
Copy link
Contributor Author

I think this is ready for another review

@shimwell
Copy link
Contributor Author

🎉 CI passed

@thomasms
Copy link
Member

Alright I guess this works. LGTM.

@thomasms thomasms merged commit f0571ca into fispact:master May 26, 2025
4 checks passed
@shimwell
Copy link
Contributor Author

Thanks for the merge, I'm already making use of this feature over here jbae11/openmc_activator#11

@shimwell
Copy link
Contributor Author

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?

@thomasms
Copy link
Member

thomasms commented Jun 1, 2025

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.

@shimwell
Copy link
Contributor Author

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

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