Skip to content
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

Get_errorr update #187

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

AitorBengoechea
Copy link
Contributor

Errorr.py:
Now get_cov returns a dictionary with the keys being mf and the values of the dictionary CategoryCov object. In case there is only one mf, it returns a CategoryCov object directly.
The distinction between nubar and xs is fundamental, since it helps to recognise errors more easily (in this case, it has helped to recognise errors in njoy.py).
On the other hand, in nucleus with more than one mf, the mf argument allows us to select which mf we will receive as output.

Njoy.py:
Bug: Previously, if the user entered a mt, that mt was applied to all error output. This generated null files or errors when parsing mt=456 with mf=33 or mt=2 with mf=31, to give an example. Now, if xs=True and we input different mt, they are parsed and if they are all greater than 450, 3/ is automatically written, so nubar and xs are parsed in the same input.

@AitorBengoechea
Copy link
Contributor Author

The program is tested for U-238, which contains mf=31, 33, 34 and 35. In the case of mf=34 and mf=35, a covariance matrix is obtained, but they are not tested (the note in the docstring explains this).

@luca-fiorito-11
Copy link
Owner

The program is tested for U-238, which contains mf=31, 33, 34 and 35. In the case of mf=34 and mf=35, a covariance matrix is obtained, but they are not tested (the note in the docstring explains this).

can you add this test to the notebooks folder?

Bengoechea Aitor added 2 commits April 26, 2022 16:03
@AitorBengoechea
Copy link
Contributor Author

Bug solve: Now when we take all the available mf, never returns a empty list.

@AitorBengoechea
Copy link
Contributor Author

NJOY now processes the mt selection as it should, since before, when a mt is introduced, it was copied to all the mf. Therefore, it could give the option that with mf=31 we were asking NJOY to process mt=2, so the program fails

@AitorBengoechea
Copy link
Contributor Author

I have added an extra condition in _group_input and _errorr_input so that if the user enters the wrong mt, they are corrected or at least not repeated. (With test include)

@@ -1868,7 +1868,13 @@ def get_errorr(self,
12 /
1.00000e-05 3.00000e-02 5.80000e-02 1.40000e-01 2.80000e-01 3.50000e-01 6.25000e-01 4.00000e+00 4.80520e+01 5.53000e+03 8.21000e+05 2.23100e+06 1.00000e+07 /
3/
3 452 'nu' /
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no test for MT and groupr together

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the method get_gendf: Line 2339
Bad mt selection in _errorr_input: line 761
Bad mt selection in _groupr_input: line 1044

The only missing test is on get_errorr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 2021: Keywords mt and groupr are incompatible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most logical option, in my opinion, is to process all the mt and do the following: a warning saying that the mt will have to be processed by the user or create lines of code at the end that filter the mt

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then let's just make a test showing the incompatibility and let it be for the moment

sandy/errorr.py Outdated Show resolved Hide resolved
sandy/njoy.py Show resolved Hide resolved
sandy/njoy.py Outdated Show resolved Hide resolved
sandy/njoy.py Outdated Show resolved Hide resolved
@@ -1868,7 +1868,13 @@ def get_errorr(self,
12 /
1.00000e-05 3.00000e-02 5.80000e-02 1.40000e-01 2.80000e-01 3.50000e-01 6.25000e-01 4.00000e+00 4.80520e+01 5.53000e+03 8.21000e+05 2.23100e+06 1.00000e+07 /
3/
3 452 'nu' /
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then let's just make a test showing the incompatibility and let it be for the moment

sandy/njoy.py Outdated

Test of wrong mt number:
>>> with pytest.raises(SyntaxError): sandy.njoy._errorr_input(20, 21, 0, 22, 9237, mt=455)
>>> print(sandy.njoy._errorr_input(20, 21, 0, 22, 9237, mt=[102,455]))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we get a error/warning here?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just raise a error right away if one of the banned_mt is found in the mt list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes that the user knows how NJOY is programmed in sandy, which is not always the case. The current programming only raises an error if we are absolutely sure that NJOY is going to fail.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid making assumptions for the users. If some MT numbers are forbidden, then they must be at all times. In this case sometimes a MT is forbidden and sometimes it is not.

sandy/njoy.py Outdated
0/

Test the wrong mt number:
>>> print(sandy.njoy._groupr_input(20, 21, 0, 22, 9237, mt=[455]))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has to raise an errorr

sandy/njoy.py Outdated Show resolved Hide resolved
sandy/njoy.py Outdated Show resolved Hide resolved
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